Revert change

This reverts commit a4720b03ee.

CHANGELOG.md

@@ -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

@@ -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

@@ -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

@@ -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

@@ -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

@@ -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

@@ -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

@@ -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

@@ -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

@@ -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 [