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

Fixed CRLF command smuggling and node misrouting in AbstractAggregateConnection::write() and CommandInterface::deserializeCommand() (#1728)

Browse Source 
* Fixed CRLF command injection / smuggling in AbstractAggregateConnection::write()

* Revert "Fixed CRLF command injection / smuggling in AbstractAggregateConnection::write()"

This reverts commit a4720b03ee.

* Fixed CRLF command smuggling and node misrouting in AbstractAggregateConnection::write() and CommandInterface::deserializeCommand()

* Revert change
This commit is contained in:
Vladyslav Vildanov
2026-09-16 10:01:26 +03:00
committed by GitHub
parent 37490865cc
commit 3edc442e55
10 changed files with 421 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files
CHANGELOG.md
+1
View File
@@ -8,6 +8,7 @@
- Fixed RESP3 double parsing returning positive `INF` for `-inf` payloads (#1716)
- Fixed `client_info` connection parameter being ignored (#1722)
- Fixed `Stream::write()`/`read()` leaving a dead connection when a host error handler throws exception (#1725)
- Fixed CRLF command smuggling and node misrouting in `AbstractAggregateConnection::write()` and `CommandInterface::deserializeCommand()` (CVE GHSA-w6f5-v2h6-g786)
## v3.6.0 (2026-08-14)
### Added
src/Command/Command.php
+48 -14
View File
@@ -164,12 +164,8 @@ abstract class Command implements CommandInterface
*/
public static function deserializeCommand(string $serializedCommand): CommandInterface
{
if ($serializedCommand[0] !== '*') {
throw new UnexpectedValueException('Invalid serializing format');
}
$commandArray = explode("\r\n", $serializedCommand);
$commandId = $commandArray[2];
$items = self::parseMultibulk($serializedCommand);
$commandId = $items[0];
$classPath = __NAMESPACE__ . '\Redis\\';
// Check if given command is a module command.
@@ -189,15 +185,53 @@ abstract class Command implements CommandInterface
}
$command = new $classPath();
$arguments = [];
for ($i = 4, $iMax = count($commandArray); $i < $iMax; $i++) {
$arguments[] = $commandArray[$i];
++$i;
}
$command->setArguments($arguments);
$command->setArguments(array_slice($items, 1));
return $command;
}
/**
* Parses a RESP multibulk buffer into its individual bulk-string values
* (command ID followed by its arguments), walking each string by its own
* declared byte length instead of splitting the buffer on "\r\n" -- which
* a bulk string's payload may legitimately contain (see GHSA-w6f5-v2h6-g786).
*
* @param string $buffer
* @return string[]
*/
private static function parseMultibulk(string $buffer): array
{
if ($buffer[0] !== '*') {
throw new UnexpectedValueException('Invalid serializing format');
}
$lineEnd = strpos($buffer, "\r\n");
if ($lineEnd === false) {
throw new UnexpectedValueException('Invalid serializing format');
}
$count = (int) substr($buffer, 1, $lineEnd - 1);
$offset = $lineEnd + 2;
$items = [];
for ($i = 0; $i < $count; ++$i) {
if (($buffer[$offset] ?? '') !== '$') {
throw new UnexpectedValueException('Invalid serializing format');
}
$lineEnd = strpos($buffer, "\r\n", $offset);
if ($lineEnd === false) {
throw new UnexpectedValueException('Invalid serializing format');
}
$bulkLen = (int) substr($buffer, $offset + 1, $lineEnd - $offset - 1);
$dataStart = $lineEnd + 2;
$items[] = substr($buffer, $dataStart, $bulkLen);
$offset = $dataStart + $bulkLen + 2;
}
return $items;
}
}
src/Command/RawCommand.php
+48 -14
View File
@@ -161,12 +161,8 @@ final class RawCommand implements CommandInterface
*/
public static function deserializeCommand(string $serializedCommand): CommandInterface
{
if ($serializedCommand[0] !== '*') {
throw new UnexpectedValueException('Invalid serializing format');
}
$commandArray = explode("\r\n", $serializedCommand);
$commandId = $commandArray[2];
$items = self::parseMultibulk($serializedCommand);
$commandId = $items[0];
$classPath = __NAMESPACE__ . '\Redis\\';
// Check if given command is a module command.
@@ -186,15 +182,53 @@ final class RawCommand implements CommandInterface
}
$command = new $classPath();
$arguments = [];
for ($i = 4, $iMax = count($commandArray); $i < $iMax; $i++) {
$arguments[] = $commandArray[$i];
++$i;
}
$command->setArguments($arguments);
$command->setArguments(array_slice($items, 1));
return $command;
}
/**
* Parses a RESP multibulk buffer into its individual bulk-string values
* (command ID followed by its arguments), walking each string by its own
* declared byte length instead of splitting the buffer on "\r\n" -- which
* a bulk string's payload may legitimately contain (see GHSA-w6f5-v2h6-g786).
*
* @param string $buffer
* @return string[]
*/
private static function parseMultibulk(string $buffer): array
{
if ($buffer[0] !== '*') {
throw new UnexpectedValueException('Invalid serializing format');
}
$lineEnd = strpos($buffer, "\r\n");
if ($lineEnd === false) {
throw new UnexpectedValueException('Invalid serializing format');
}
$count = (int) substr($buffer, 1, $lineEnd - 1);
$offset = $lineEnd + 2;
$items = [];
for ($i = 0; $i < $count; ++$i) {
if (($buffer[$offset] ?? '') !== '$') {
throw new UnexpectedValueException('Invalid serializing format');
}
$lineEnd = strpos($buffer, "\r\n", $offset);
if ($lineEnd === false) {
throw new UnexpectedValueException('Invalid serializing format');
}
$bulkLen = (int) substr($buffer, $offset + 1, $lineEnd - $offset - 1);
$dataStart = $lineEnd + 2;
$items[] = substr($buffer, $dataStart, $bulkLen);
$offset = $dataStart + $bulkLen + 2;
}
return $items;
}
}
src/Connection/AbstractAggregateConnection.php
+17 -9
View File
@@ -77,17 +77,25 @@ abstract class AbstractAggregateConnection implements AggregateConnectionInterfa
*/
public function write(string $buffer): void
{
$rawCommands = [];
$explodedBuffer = explode("\r\n", trim($buffer));
$offset = 0;
$length = strlen($buffer);
while (!empty($explodedBuffer)) {
$argsLen = (int) explode('*', $explodedBuffer[0])[1];
$cmdLen = ($argsLen * 2) + 1;
$rawCommands[] = array_splice($explodedBuffer, 0, $cmdLen);
}
while ($offset < $length) {
$start = $offset;
$lineEnd = strpos($buffer, "\r\n", $offset);
$argsCount = (int) substr($buffer, $offset + 1, $lineEnd - $offset - 1);
$offset = $lineEnd + 2;
foreach ($rawCommands as $command) {
$command = implode("\r\n", $command) . "\r\n";
// Advance by each bulk string's own declared byte length rather than
// splitting on literal "\r\n", which a bulk string's value may legitimately
// contain (see GHSA-w6f5-v2h6-g786).
for ($i = 0; $i < $argsCount; ++$i) {
$lineEnd = strpos($buffer, "\r\n", $offset);
$bulkLen = (int) substr($buffer, $offset + 1, $lineEnd - $offset - 1);
$offset = $lineEnd + 2 + $bulkLen + 2;
}
$command = substr($buffer, $start, $offset - $start);
$commandObj = Command::deserializeCommand($command);
$this->getConnectionByCommand($commandObj)->write($command);
}
tests/Predis/Command/CommandTest.php
+11
View File
@@ -281,6 +281,17 @@ class CommandTest extends PredisTestCase
TOPKQUERY::class,
['key'],
],
// Regression cases for GHSA-w6f5-v2h6-g786 (CWE-93): a "\r\n" embedded
// in a bulk string's own byte payload must not be mistaken for a RESP
// line boundary.
'GET with CRLF embedded in the value' => [
GET::class,
["value\r\n*1\r\n\$4\r\nEVIL"],
],
'ZADD with CRLF embedded in the key' => [
ZADD::class,
["key\r\n*1\r\n\$4\r\nEVIL", 'value', 'key1', 'value1'],
],
];
}
}
tests/Predis/Command/RawCommandTest.php
+11
View File
@@ -238,6 +238,17 @@ class RawCommandTest extends PredisTestCase
TOPKQUERY::class,
['key'],
],
// Regression cases for GHSA-w6f5-v2h6-g786 (CWE-93): a "\r\n" embedded
// in a bulk string's own byte payload must not be mistaken for a RESP
// line boundary.
'GET with CRLF embedded in the value' => [
GET::class,
["value\r\n*1\r\n\$4\r\nEVIL"],
],
'ZADD with CRLF embedded in the key' => [
ZADD::class,
["key\r\n*1\r\n\$4\r\nEVIL", 'value', 'key1', 'value1'],
],
];
}
}
tests/Predis/Connection/Cluster/PredisClusterTest.php
+70
View File
@@ -14,6 +14,7 @@ namespace Predis\Connection\Cluster;
use Predis\Command\CommandInterface;
use Predis\Command\Redis\GET;
use Predis\Command\Redis\SET;
use Predis\Connection\Parameters;
use PredisTestCase;
@@ -516,4 +517,73 @@ class PredisClusterTest extends PredisTestCase
$cluster->write($command1->serializeCommand() . $command2->serializeCommand() . $command3->serializeCommand());
}
/**
* Regression guard for GHSA-w6f5-v2h6-g786 (CWE-93): a CRLF embedded in a
* bulk string's own value must not be mistaken for a command boundary
* (splitting one command into a smuggled extra command), and must not
* corrupt the argument list used to pick the target node.
*
* @group disconnected
*/
public function testWriteHandlesCRLFEmbeddedInBulkStringValue(): void
{
$command = new SET();
$command->setArguments(['victim-key', "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');
$cluster = new PredisCluster(new Parameters());
$cluster->add($connection1);
$cluster->add($connection2);
$expectedConnection = $cluster->getConnectionByCommand($command);
$otherConnection = $expectedConnection === $connection1 ? $connection2 : $connection1;
$expectedConnection
->expects($this->once())
->method('write')
->with($command->serializeCommand());
$otherConnection
->expects($this->never())
->method('write');
$cluster->write($command->serializeCommand());
}
/**
* Regression guard for GHSA-w6f5-v2h6-g786 (CWE-93): a CRLF embedded in a
* bulk string KEY must not corrupt the argument list used to pick the
* target node, which would silently route the command to the wrong node.
*
* @group disconnected
*/
public function testWriteHandlesCRLFEmbeddedInBulkStringKey(): void
{
$command = new SET();
$command->setArguments(["victim\r\n*1\r\n\$4\r\nEVIL", 'somevalue']);
$connection1 = $this->getMockConnection('tcp://127.0.0.1:7001');
$connection2 = $this->getMockConnection('tcp://127.0.0.1:7002');
$cluster = new PredisCluster(new Parameters());
$cluster->add($connection1);
$cluster->add($connection2);
$expectedConnection = $cluster->getConnectionByCommand($command);
$otherConnection = $expectedConnection === $connection1 ? $connection2 : $connection1;
$expectedConnection
->expects($this->once())
->method('write')
->with($command->serializeCommand());
$otherConnection
->expects($this->never())
->method('write');
$cluster->write($command->serializeCommand());
}
}
tests/Predis/Connection/Cluster/RedisClusterTest.php
+73
View File
@@ -1676,4 +1676,77 @@ class RedisClusterTest extends PredisTestCase
$cluster->write($command1->serializeCommand() . $command2->serializeCommand() . $command3->serializeCommand());
}
/**
* Regression guard for GHSA-w6f5-v2h6-g786 (CWE-93): a CRLF embedded in a
* bulk string's own value must not be mistaken for a command boundary
* (splitting one command into a smuggled extra command), and must not
* corrupt the argument list used to pick the target node.
*
* @group disconnected
*/
public function testWriteHandlesCRLFEmbeddedInBulkStringValue(): void
{
$command = new Command\Redis\SET();
$command->setArguments(['victim-key', "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');
$cluster = new RedisCluster($factory, new Parameters());
$cluster->add($connection1);
$cluster->add($connection2);
$cluster->add($connection3);
$expectedConnection = $cluster->getConnectionByCommand($command);
foreach ([$connection1, $connection2, $connection3] as $connection) {
if ($connection === $expectedConnection) {
$connection->expects($this->once())->method('write')->with($command->serializeCommand());
} else {
$connection->expects($this->never())->method('write');
}
}
$cluster->write($command->serializeCommand());
}
/**
* Regression guard for GHSA-w6f5-v2h6-g786 (CWE-93): a CRLF embedded in a
* bulk string KEY must not corrupt the argument list used to pick the
* target node, which would silently route the command to the wrong node.
*
* @group disconnected
*/
public function testWriteHandlesCRLFEmbeddedInBulkStringKey(): void
{
$command = new Command\Redis\SET();
$command->setArguments(["victim\r\n*1\r\n\$4\r\nEVIL", 'somevalue']);
$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');
$cluster = new RedisCluster($factory, new Parameters());
$cluster->add($connection1);
$cluster->add($connection2);
$cluster->add($connection3);
$expectedConnection = $cluster->getConnectionByCommand($command);
foreach ([$connection1, $connection2, $connection3] as $connection) {
if ($connection === $expectedConnection) {
$connection->expects($this->once())->method('write')->with($command->serializeCommand());
} else {
$connection->expects($this->never())->method('write');
}
}
$cluster->write($command->serializeCommand());
}
}
tests/Predis/Connection/Replication/MasterSlaveReplicationTest.php
+63
View File
@@ -1496,6 +1496,69 @@ repl_backlog_histlen:12978
$replication->write($command1->serializeCommand() . $command2->serializeCommand() . $command3->serializeCommand());
}
/**
* Regression guard for GHSA-w6f5-v2h6-g786 (CWE-93): a CRLF embedded in a
* bulk string's own value must not be mistaken for a command boundary
* (splitting one command into a smuggled extra command).
*
* @group disconnected
*/
public function testWriteHandlesCRLFEmbeddedInBulkStringValue(): void
{
$command = new Command\Redis\SET();
$command->setArguments(['victim-key', "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
->expects($this->never())
->method('write');
$master
->expects($this->once())
->method('write')
->with($command->serializeCommand());
$replication = new MasterSlaveReplication();
$replication->add($master);
$replication->add($slave1);
$replication->write($command->serializeCommand());
}
/**
* Regression guard for GHSA-w6f5-v2h6-g786 (CWE-93): a CRLF embedded in a
* bulk string KEY must not corrupt the argument list used to pick the
* target connection, which would silently route the command to the
* wrong node.
*
* @group disconnected
*/
public function testWriteHandlesCRLFEmbeddedInBulkStringKey(): void
{
$command = new Command\Redis\SET();
$command->setArguments(["victim\r\n*1\r\n\$4\r\nEVIL", 'somevalue']);
$master = $this->getMockConnection('tcp://127.0.0.1:6379?role=master');
$slave1 = $this->getMockConnection('tcp://127.0.0.1:6380?role=slave');
$slave1
->expects($this->never())
->method('write');
$master
->expects($this->once())
->method('write')
->with($command->serializeCommand());
$replication = new MasterSlaveReplication();
$replication->add($master);
$replication->add($slave1);
$replication->write($command->serializeCommand());
}
/**
* @medium
* @group disconnected
tests/Predis/Connection/Replication/SentinelReplicationTest.php
+79
View File
@@ -2020,6 +2020,85 @@ class SentinelReplicationTest extends PredisTestCase
$replication->write($command1->serializeCommand() . $command2->serializeCommand() . $command3->serializeCommand());
}
/**
* Regression guard for GHSA-w6f5-v2h6-g786 (CWE-93): a CRLF embedded in a
* bulk string's own value must not be mistaken for a command boundary
* (splitting one command into a smuggled extra command).
*
* @group disconnected
*/
public function testWriteHandlesCRLFEmbeddedInBulkStringValue(): void
{
$command = new Command\Redis\SET();
$command->setArguments(['victim-key', "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');
$slave = $this->getMockConnection('tcp://127.0.0.1:6380?role=slave');
$strategy = new Replication\ReplicationStrategy();
$factory = new Connection\Factory();
$master
->expects($this->once())
->method('isConnected')
->willReturn(true);
$slave
->expects($this->never())
->method('write');
$master
->expects($this->once())
->method('write')
->with($command->serializeCommand());
$replication = new SentinelReplication('svc', [$sentinel], $factory, $strategy);
$replication->add($master);
$replication->add($slave);
$replication->write($command->serializeCommand());
}
/**
* Regression guard for GHSA-w6f5-v2h6-g786 (CWE-93): a CRLF embedded in a
* bulk string KEY must not corrupt the argument list used to pick the
* target connection, which would silently route the command to the
* wrong node.
*
* @group disconnected
*/
public function testWriteHandlesCRLFEmbeddedInBulkStringKey(): void
{
$command = new Command\Redis\SET();
$command->setArguments(["victim\r\n*1\r\n\$4\r\nEVIL", 'somevalue']);
$sentinel = $this->getMockSentinelConnection('tcp://127.0.0.1:5381?role=sentinel');
$master = $this->getMockConnection('tcp://127.0.0.1:6379?role=master');
$slave = $this->getMockConnection('tcp://127.0.0.1:6380?role=slave');
$strategy = new Replication\ReplicationStrategy();
$factory = new Connection\Factory();
$master
->expects($this->once())
->method('isConnected')
->willReturn(true);
$slave
->expects($this->never())
->method('write');
$master
->expects($this->once())
->method('write')
->with($command->serializeCommand());
$replication = new SentinelReplication('svc', [$sentinel], $factory, $strategy);
$replication->add($master);
$replication->add($slave);
$replication->write($command->serializeCommand());
}
public function connectionsProvider(): array
{
return [