Validate target URI so hostname can not contain excessive URI components

Author: clue

src/Server.php

@@ -7,14 +7,12 @@
 use React\Promise;
 use React\Promise\Deferred;
 use React\Promise\PromiseInterface;
-use React\Promise\CancellablePromiseInterface;
 use React\Socket\ConnectorInterface;
 use React\Socket\Connector;
 use React\Socket\ConnectionInterface;
 use React\EventLoop\LoopInterface;
 use \UnexpectedValueException;
 use \InvalidArgumentException;
-use \RuntimeException;
 use \Exception;
 
 class Server extends EventEmitter
@@ -326,6 +324,12 @@ public function connectTarget(ConnectionInterface $stream, array $target)
         }
         $uri = $uri . ':' . $target[1];
 
+        // validate URI so a string hostname can not pass excessive URI parts
+        $parts = parse_url('tcp://' . $uri);
+        if (!$parts || !isset($parts['scheme'], $parts['host'], $parts['port']) || count($parts) !== 3) {
+            return Promise\reject(new InvalidArgumentException('Invalid target URI given'));
+        }
+
         $stream->emit('target', $target);
         $that = $this;
         $connecting = $this->connector->connect($uri);

tests/ServerTest.php

@@ -152,7 +152,7 @@ public function testHandleSocksConnectionWillEndOnInvalidData()
         $connection->emit('data', array('asdasdasdasdasd'));
     }
 
-    public function testHandleSocks4ConnectionWillEstablishOutgoingConnection()
+    public function testHandleSocks4ConnectionWithIpv4WillEstablishOutgoingConnection()
     {
         $connection = $this->getMockBuilder('React\Socket\Connection')->disableOriginalConstructor()->setMethods(array('pause', 'end'))->getMock();
 
@@ -165,7 +165,31 @@ public function testHandleSocks4ConnectionWillEstablishOutgoingConnection()
         $connection->emit('data', array("\x04\x01" . "\x00\x50" . pack('N', ip2long('127.0.0.1')) . "\x00"));
     }
 
-    public function testHandleSocks5ConnectionWillEstablishOutgoingConnection()
+    public function testHandleSocks4aConnectionWithHostnameWillEstablishOutgoingConnection()
+    {
+        $connection = $this->getMockBuilder('React\Socket\Connection')->disableOriginalConstructor()->setMethods(array('pause', 'end'))->getMock();
+
+        $promise = new Promise(function () { });
+
+        $this->connector->expects($this->once())->method('connect')->with('example.com:80')->willReturn($promise);
+
+        $this->server->onConnection($connection);
+
+        $connection->emit('data', array("\x04\x01" . "\x00\x50" . "\x00\x00\x00\x01" . "\x00" . "example.com" . "\x00"));
+    }
+
+    public function testHandleSocks4aConnectionWithInvalidHostnameWillNotEstablishOutgoingConnection()
+    {
+        $connection = $this->getMockBuilder('React\Socket\Connection')->disableOriginalConstructor()->setMethods(array('pause', 'end'))->getMock();
+
+        $this->connector->expects($this->never())->method('connect');
+
+        $this->server->onConnection($connection);
+
+        $connection->emit('data', array("\x04\x01" . "\x00\x50" . "\x00\x00\x00\x01" . "\x00" . "tls://example.com:80?" . "\x00"));
+    }
+
+    public function testHandleSocks5ConnectionWithIpv4WillEstablishOutgoingConnection()
     {
         $connection = $this->getMockBuilder('React\Socket\Connection')->disableOriginalConstructor()->setMethods(array('pause', 'end', 'write'))->getMock();
 
@@ -191,6 +215,30 @@ public function testHandleSocks5ConnectionWithIpv6WillEstablishOutgoingConnectio
         $connection->emit('data', array("\x05\x01\x00" . "\x05\x01\x00\x04" . inet_pton('::1') . "\x00\x50"));
     }
 
+    public function testHandleSocks5ConnectionWithHostnameWillEstablishOutgoingConnection()
+    {
+        $connection = $this->getMockBuilder('React\Socket\Connection')->disableOriginalConstructor()->setMethods(array('pause', 'end', 'write'))->getMock();
+
+        $promise = new Promise(function () { });
+
+        $this->connector->expects($this->once())->method('connect')->with('example.com:80')->willReturn($promise);
+
+        $this->server->onConnection($connection);
+
+        $connection->emit('data', array("\x05\x01\x00" . "\x05\x01\x00\x03\x0B" . "example.com" . "\x00\x50"));
+    }
+
+    public function testHandleSocks5ConnectionWithInvalidHostnameWillNotEstablishOutgoingConnection()
+    {
+        $connection = $this->getMockBuilder('React\Socket\Connection')->disableOriginalConstructor()->setMethods(array('pause', 'end', 'write'))->getMock();
+
+        $this->connector->expects($this->never())->method('connect');
+
+        $this->server->onConnection($connection);
+
+        $connection->emit('data', array("\x05\x01\x00" . "\x05\x01\x00\x03\x15" . "tls://example.com:80?" . "\x00\x50"));
+    }
+
     public function testHandleSocksConnectionWillCancelOutputConnectionIfIncomingCloses()
     {
         $connection = $this->getMockBuilder('React\Socket\Connection')->disableOriginalConstructor()->setMethods(array('pause', 'end'))->getMock();