geckon01/predis
1
0
Fork 0
mirror of https://github.com/predis/predis.git synced 2026-09-12 11:27:05 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fixed CRLF command injection / smuggling in AbstractAggregateConnection::write() (#1719)

Browse Source
This commit is contained in:
Vladyslav Vildanov
2026-09-11 17:58:18 +03:00
committed by GitHub
parent 3ffaf10e99
commit 8ee8993eb7
11 changed files with 75 additions and 402 deletions
Download Patch File Download Diff File Expand all files Collapse all files
CHANGELOG.md
+3
View File
@@ -3,6 +3,9 @@
## Unreleased
### Changed
- Changed RESP3 double parsing to return `NAN` for NaN payloads instead of `0.0`
### Fixed
- Fixed CRLF command injection / smuggling in `AbstractAggregateConnection::write()` (CVE GHSA-w6f5-v2h6-g786, CWE-93)
- Fixed RESP3 double parsing returning positive `INF` for `-inf` payloads (#1716)
## v3.6.0 (2026-08-14)
### Added
src/Command/Command.php
-45
View File
@@ -12,9 +12,6 @@
namespace Predis\Command;
use Predis\ClientConfiguration;
use UnexpectedValueException;
/**
* Base class for Redis commands.
*/
@@ -155,46 +152,4 @@ abstract class Command implements CommandInterface
return $buffer;
}
/**
* {@inheritDoc}
*/
public static function deserializeCommand(string $serializedCommand): CommandInterface
{
if ($serializedCommand[0] !== '*') {
throw new UnexpectedValueException('Invalid serializing format');
}
$commandArray = explode("\r\n", $serializedCommand);
$commandId = $commandArray[2];
$classPath = __NAMESPACE__ . '\Redis\\';
// Check if given command is a module command.
if (count($commandIdArray = explode('.', $commandId)) > 1) {
// Fetch module configuration to resolve namespace.
$moduleConfiguration = array_filter(
ClientConfiguration::getModules(),
static function ($module) use ($commandIdArray) {
return $module['commandPrefix'] === $commandIdArray[0];
}
);
$commandClass = strtoupper($commandIdArray[0] . $commandIdArray[1]);
$classPath .= array_shift($moduleConfiguration)['name'] . '\\' . $commandClass;
} else {
$classPath .= $commandIdArray[0];
}
$command = new $classPath();
$arguments = [];
for ($i = 4, $iMax = count($commandArray); $i < $iMax; $i++) {
$arguments[] = $commandArray[$i];
++$i;
}
$command->setArguments($arguments);
return $command;
}
}
src/Command/CommandInterface.php
-8
View File
@@ -92,12 +92,4 @@ interface CommandInterface
* @return string
*/
public function serializeCommand(): string;
/**
* Creates command object from given serialized representation.
*
* @param string $serializedCommand
* @return static
*/
public static function deserializeCommand(string $serializedCommand): CommandInterface;
}
src/Command/RawCommand.php
-42
View File
@@ -12,9 +12,6 @@
namespace Predis\Command;
use Predis\ClientConfiguration;
use UnexpectedValueException;
/**
* Class representing a generic Redis command.
*
@@ -152,43 +149,4 @@ final class RawCommand implements CommandInterface
return $buffer;
}
public static function deserializeCommand(string $serializedCommand): CommandInterface
{
if ($serializedCommand[0] !== '*') {
throw new UnexpectedValueException('Invalid serializing format');
}
$commandArray = explode("\r\n", $serializedCommand);
$commandId = $commandArray[2];
$classPath = __NAMESPACE__ . '\Redis\\';
// Check if given command is a module command.
if (count($commandIdArray = explode('.', $commandId)) > 1) {
// Fetch module configuration to resolve namespace.
$moduleConfiguration = array_filter(
ClientConfiguration::getModules(),
static function ($module) use ($commandIdArray) {
return $module['commandPrefix'] === $commandIdArray[0];
}
);
$commandClass = strtoupper($commandIdArray[0] . $commandIdArray[1]);
$classPath .= array_shift($moduleConfiguration)['name'] . '\\' . $commandClass;
} else {
$classPath .= $commandIdArray[0];
}
$command = new $classPath();
$arguments = [];
for ($i = 4, $iMax = count($commandArray); $i < $iMax; $i++) {
$arguments[] = $commandArray[$i];
++$i;
}
$command->setArguments($arguments);
return $command;
}
}
src/Connection/AbstractAggregateConnection.php
+8 -15
View File
@@ -12,8 +12,8 @@
namespace Predis\Connection;
use Predis\Command\Command;
use Predis\Command\CommandInterface;
use Predis\NotSupportedException;
abstract class AbstractAggregateConnection implements AggregateConnectionInterface
{
@@ -77,19 +77,12 @@ abstract class AbstractAggregateConnection implements AggregateConnectionInterfa
*/
public function write(string $buffer): void
{
$rawCommands = [];
$explodedBuffer = explode("\r\n", trim($buffer));
while (!empty($explodedBuffer)) {
$argsLen = (int) explode('*', $explodedBuffer[0])[1];
$cmdLen = ($argsLen * 2) + 1;
$rawCommands[] = array_splice($explodedBuffer, 0, $cmdLen);
}
foreach ($rawCommands as $command) {
$command = implode("\r\n", $command) . "\r\n";
$commandObj = Command::deserializeCommand($command);
$this->getConnectionByCommand($commandObj)->write($command);
}
// Refuse raw buffers: re-splitting them on "\r\n" ignored RESP length
// prefixes and let CRLF-smuggled commands be routed to a node
// (CVE GHSA-w6f5-v2h6-g786). Pipelines write each command individually.
throw new NotSupportedException(
'Aggregate connections cannot write a raw command buffer; '
. 'route each command through writeRequest() instead.'
);
}
}
tests/Predis/Command/CommandTest.php
-104
View File
@@ -12,23 +12,8 @@
namespace Predis\Command;
use Predis\Command\Redis\BloomFilter\BFADD;
use Predis\Command\Redis\CountMinSketch\CMSINFO;
use Predis\Command\Redis\CuckooFilter\CFADD;
use Predis\Command\Redis\GET;
use Predis\Command\Redis\Json\JSONSET;
use Predis\Command\Redis\MGET;
use Predis\Command\Redis\Search\FTAGGREGATE;
use Predis\Command\Redis\Search\FTEXPLAIN;
use Predis\Command\Redis\Search\FTSEARCH;
use Predis\Command\Redis\Search\FTSPELLCHECK;
use Predis\Command\Redis\TDigest\TDIGESTADD;
use Predis\Command\Redis\TimeSeries\TSGET;
use Predis\Command\Redis\TopK\TOPKQUERY;
use Predis\Command\Redis\ZADD;
use PredisTestCase;
use stdClass;
use UnexpectedValueException;
class CommandTest extends PredisTestCase
{
@@ -194,93 +179,4 @@ class CommandTest extends PredisTestCase
$command->serializeCommand()
);
}
/**
* @dataProvider deserializeCommandProvider
* @group disconnected
*/
public function testDeserializeCommand(string $class, array $arguments): void
{
$command = new $class();
$command->setArguments($arguments);
$deserializedCommand = Command::deserializeCommand($command->serializeCommand());
$this->assertInstanceOf($class, $deserializedCommand);
$this->assertEquals($command->getArguments(), $deserializedCommand->getArguments());
}
/**
* @group disconnected
* @return void
*/
public function testDeserializeCommandThrowsException(): void
{
$this->expectException(UnexpectedValueException::class);
$this->expectExceptionMessage('Invalid serializing format');
Command::deserializeCommand('foobar');
}
public function deserializeCommandProvider(): array
{
return [
'GET' => [
GET::class,
['arg1'],
],
'MGET' => [
MGET::class,
['arg1', 'arg2', 'arg3'],
],
'ZADD' => [
ZADD::class,
['key', 'value', 'key1', 'value1'],
],
'JSONSET' => [
JSONSET::class,
['key', '$', '{"key":"value"}'],
],
'BFADD' => [
BFADD::class,
['key', 'value'],
],
'CFADD' => [
CFADD::class,
['key', 'value'],
],
'CMSINFO' => [
CMSINFO::class,
['key', 'value'],
],
'FTSEARCH' => [
FTSEARCH::class,
['key', 'value', 'DIALECT', '2'],
],
'FTAGGREGATE' => [
FTAGGREGATE::class,
['key', 'value', 'DIALECT', '2'],
],
'FTSPELLCHECK' => [
FTSPELLCHECK::class,
['key', 'value', 'DIALECT', '2'],
],
'FTEXPLAIN' => [
FTEXPLAIN::class,
['key', 'value', 'DIALECT', '2'],
],
'TDIGESTADD' => [
TDIGESTADD::class,
['key', 'value'],
],
'TSGET' => [
TSGET::class,
['key'],
],
'TOPKQUERY' => [
TOPKQUERY::class,
['key'],
],
];
}
}
tests/Predis/Command/RawCommandTest.php
-89
View File
@@ -12,19 +12,7 @@
namespace Predis\Command;
use Predis\Command\Redis\BloomFilter\BFADD;
use Predis\Command\Redis\CountMinSketch\CMSINFO;
use Predis\Command\Redis\CuckooFilter\CFADD;
use Predis\Command\Redis\GET;
use Predis\Command\Redis\Json\JSONSET;
use Predis\Command\Redis\MGET;
use Predis\Command\Redis\Search\FTSEARCH;
use Predis\Command\Redis\TDigest\TDIGESTADD;
use Predis\Command\Redis\TimeSeries\TSGET;
use Predis\Command\Redis\TopK\TOPKQUERY;
use Predis\Command\Redis\ZADD;
use PredisTestCase;
use UnexpectedValueException;
class RawCommandTest extends PredisTestCase
{
@@ -163,81 +151,4 @@ class RawCommandTest extends PredisTestCase
$command->serializeCommand()
);
}
/**
* @dataProvider deserializeCommandProvider
* @group disconnected
*/
public function testDeserializeCommand(string $class, array $arguments): void
{
$command = new $class();
$command->setArguments($arguments);
$deserializedCommand = RawCommand::deserializeCommand($command->serializeCommand());
$this->assertInstanceOf($class, $deserializedCommand);
$this->assertEquals($command->getArguments(), $deserializedCommand->getArguments());
}
/**
* @group disconnected
* @return void
*/
public function testDeserializeCommandThrowsException(): void
{
$this->expectException(UnexpectedValueException::class);
$this->expectExceptionMessage('Invalid serializing format');
RawCommand::deserializeCommand('foobar');
}
public function deserializeCommandProvider(): array
{
return [
'GET' => [
GET::class,
['arg1'],
],
'MGET' => [
MGET::class,
['arg1', 'arg2', 'arg3'],
],
'ZADD' => [
ZADD::class,
['key', 'value', 'key1', 'value1'],
],
'JSONSET' => [
JSONSET::class,
['key', '$', '{"key":"value"}'],
],
'BFADD' => [
BFADD::class,
['key', 'value'],
],
'CFADD' => [
CFADD::class,
['key', 'value'],
],
'CMSINFO' => [
CMSINFO::class,
['key', 'value'],
],
'FTSEARCH' => [
FTSEARCH::class,
['key', 'value'],
],
'TDIGESTADD' => [
TDIGESTADD::class,
['key', 'value'],
],
'TSGET' => [
TSGET::class,
['key'],
],
'TOPKQUERY' => [
TOPKQUERY::class,
['key'],
],
];
}
}
tests/Predis/Connection/Cluster/PredisClusterTest.php
+14 -29
View File
@@ -474,46 +474,31 @@ class PredisClusterTest extends PredisTestCase
}
/**
* Regression guard for CVE GHSA-w6f5-v2h6-g786 (CWE-93): an aggregate connection
* must refuse a raw, already-serialized command buffer instead of re-splitting it
* on "\r\n". The old parser ignored RESP bulk-length prefixes, so CRLF sequences
* smuggled into a value or key were parsed as extra commands and routed to a node.
*
* @group disconnected
*/
public function testWrite(): void
public function testWriteRejectsRawCommandBuffer(): void
{
$command1 = new GET();
$command1->setArguments(['arg1']);
$command2 = new GET();
$command2->setArguments(['arg2']);
$command3 = new GET();
$command3->setArguments(['arg3']);
// A single GET whose key carries a smuggled FLUSHDB payload; the old code
// would have re-parsed and routed the FLUSHDB, this must route nothing.
$command = new GET();
$command->setArguments(["slug:PAD\r\n*1\r\n\$7\r\nFLUSHDB"]);
$connection1 = $this->getMockConnection('tcp://127.0.0.1:7001');
$connection2 = $this->getMockConnection('tcp://127.0.0.1:7002');
$connection3 = $this->getMockConnection('tcp://127.0.0.1:7003');
$connection1
->expects($this->exactly(3))
->method('write')
->withConsecutive(
[$command1->serializeCommand()],
[$command2->serializeCommand()],
[$command3->serializeCommand()]
);
$connection2
->expects($this->never())
->method('write');
$connection3
->expects($this->never())
->method('write');
$cluster = new PredisCluster(new Parameters());
$cluster->add($connection1);
$cluster->add($connection2);
$cluster->add($connection3);
$cluster->write($command1->serializeCommand() . $command2->serializeCommand() . $command3->serializeCommand());
$this->expectException('Predis\NotSupportedException');
$this->expectExceptionMessage('Aggregate connections cannot write a raw command buffer');
$cluster->write($command->serializeCommand());
}
}
tests/Predis/Connection/Cluster/RedisClusterTest.php
+16 -29
View File
@@ -1634,46 +1634,33 @@ class RedisClusterTest extends PredisTestCase
}
/**
* Regression guard for CVE GHSA-w6f5-v2h6-g786 (CWE-93): an aggregate connection
* must refuse a raw, already-serialized command buffer instead of re-splitting it
* on "\r\n". The old parser ignored RESP bulk-length prefixes, so CRLF sequences
* smuggled into a value or key were parsed as extra commands and routed to a node.
*
* @group disconnected
*/
public function testWrite(): void
public function testWriteRejectsRawCommandBuffer(): void
{
$command1 = new Command\Redis\GET();
$command1->setArguments(['arg1']);
$command2 = new Command\Redis\GET();
$command2->setArguments(['arg2']);
$command3 = new Command\Redis\GET();
$command3->setArguments(['arg3']);
// A single GET whose key carries a smuggled FLUSHDB payload; the old code
// would have re-parsed and routed the FLUSHDB, this must route nothing.
$command = new Command\Redis\GET();
$command->setArguments(["slug:PAD\r\n*1\r\n\$7\r\nFLUSHDB"]);
$factory = $this->getMockBuilder(FactoryInterface::class)->getMock();
$connection1 = $this->getMockConnection('tcp://127.0.0.1:7001');
$connection2 = $this->getMockConnection('tcp://127.0.0.1:7002');
$connection3 = $this->getMockConnection('tcp://127.0.0.1:7003');
$connection1
->expects($this->once())
->method('write')
->with($command3->serializeCommand());
$connection2
->expects($this->once())
->method('write')
->with($command2->serializeCommand());
$connection3
->expects($this->once())
->method('write')
->with($command1->serializeCommand());
->expects($this->never())
->method('write');
$cluster = new RedisCluster($factory, new Parameters());
$cluster->add($connection1);
$cluster->add($connection2);
$cluster->add($connection3);
$cluster->write($command1->serializeCommand() . $command2->serializeCommand() . $command3->serializeCommand());
$this->expectException('Predis\NotSupportedException');
$this->expectExceptionMessage('Aggregate connections cannot write a raw command buffer');
$cluster->write($command->serializeCommand());
}
}
tests/Predis/Connection/Replication/MasterSlaveReplicationTest.php
+18 -19
View File
@@ -1459,41 +1459,40 @@ repl_backlog_histlen:12978
}
/**
* Regression guard for CVE GHSA-w6f5-v2h6-g786 (CWE-93): an aggregate connection
* must refuse a raw, already-serialized command buffer instead of re-splitting it
* on "\r\n". The old parser ignored RESP bulk-length prefixes, so CRLF sequences
* smuggled into a value or key were parsed as extra commands and routed to a node.
*
* @group disconnected
*/
public function testWrite(): void
public function testWriteRejectsRawCommandBuffer(): void
{
$command1 = new Command\Redis\Json\JSONGET();
$command1->setArguments(['arg1']);
$command2 = new Command\Redis\Json\JSONGET();
$command2->setArguments(['arg2']);
$command3 = new Command\Redis\Json\JSONGET();
$command3->setArguments(['arg3']);
// A single command whose key carries a smuggled FLUSHDB payload; the old code
// would have re-parsed and routed the FLUSHDB, this must route nothing.
$command = new Command\Redis\Json\JSONGET();
$command->setArguments(["slug:PAD\r\n*1\r\n\$7\r\nFLUSHDB"]);
$master = $this->getMockConnection('tcp://127.0.0.1:6379?role=master');
$slave1 = $this->getMockConnection('tcp://127.0.0.1:6380?role=slave');
$slave1
$master
->expects($this->never())
->method('write');
$master
->expects($this->exactly(3))
->method('write')
->withConsecutive(
[$command1->serializeCommand()],
[$command2->serializeCommand()],
[$command3->serializeCommand()]
);
$slave1
->expects($this->never())
->method('write');
$replication = new MasterSlaveReplication();
$replication->add($master);
$replication->add($slave1);
$replication->write($command1->serializeCommand() . $command2->serializeCommand() . $command3->serializeCommand());
$this->expectException('Predis\NotSupportedException');
$this->expectExceptionMessage('Aggregate connections cannot write a raw command buffer');
$replication->write($command->serializeCommand());
}
/**
tests/Predis/Connection/Replication/SentinelReplicationTest.php
+16 -22
View File
@@ -1975,18 +1975,19 @@ class SentinelReplicationTest extends PredisTestCase
}
/**
* Regression guard for CVE GHSA-w6f5-v2h6-g786 (CWE-93): an aggregate connection
* must refuse a raw, already-serialized command buffer instead of re-splitting it
* on "\r\n". The old parser ignored RESP bulk-length prefixes, so CRLF sequences
* smuggled into a value or key were parsed as extra commands and routed to a node.
*
* @group disconnected
*/
public function testWrite(): void
public function testWriteRejectsRawCommandBuffer(): void
{
$command1 = new Command\Redis\Search\FTSEARCH();
$command1->setArguments(['arg1', '*']);
$command2 = new Command\Redis\Search\FTSEARCH();
$command2->setArguments(['arg2', '*']);
$command3 = new Command\Redis\Search\FTSEARCH();
$command3->setArguments(['arg3', '*']);
// A single command whose argument carries a smuggled FLUSHDB payload; the old
// code would have re-parsed and routed the FLUSHDB, this must route nothing.
$command = new Command\Redis\Search\FTSEARCH();
$command->setArguments(["idx:PAD\r\n*1\r\n\$7\r\nFLUSHDB", '*']);
$sentinel = $this->getMockSentinelConnection('tcp://127.0.0.1:5381?role=sentinel');
$master = $this->getMockConnection('tcp://127.0.0.1:6379?role=master');
@@ -1995,29 +1996,22 @@ class SentinelReplicationTest extends PredisTestCase
$factory = new Connection\Factory();
$master
->expects($this->exactly(3))
->method('isConnected')
->willReturn(true);
->expects($this->never())
->method('write');
$slave
->expects($this->never())
->method('write');
$master
->expects($this->exactly(3))
->method('write')
->withConsecutive(
[$command1->serializeCommand()],
[$command2->serializeCommand()],
[$command3->serializeCommand()]
);
$replication = new SentinelReplication('svc', [$sentinel], $factory, $strategy);
$replication->add($master);
$replication->add($slave);
$replication->write($command1->serializeCommand() . $command2->serializeCommand() . $command3->serializeCommand());
$this->expectException('Predis\NotSupportedException');
$this->expectExceptionMessage('Aggregate connections cannot write a raw command buffer');
$replication->write($command->serializeCommand());
}
public function connectionsProvider(): array