Merge pull request #59 from clue-labs/target

Fix connecting to IPv6 address via SOCKS5 and 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
@@ -320,9 +318,21 @@ public function handleSocks5(ConnectionInterface $stream, $auth=null, StreamRead
 
     public function connectTarget(ConnectionInterface $stream, array $target)
     {
+        $uri = $target[0];
+        if (strpos($uri, ':') !== false) {
+            $uri = '[' . $uri . ']';
+        }
+        $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($target[0] . ':' . $target[1]);
+        $connecting = $this->connector->connect($uri);
 
         $stream->on('close', function () use ($connecting) {
             $connecting->cancel();

tests/ServerTest.php

@@ -152,7 +152,7 @@ public function testHandleSocksConnectionWillEndOnInvalidData()
         $connection->emit('data', array('asdasdasdasdasd'));
     }
 
-    public function testHandleSocksConnectionWillEstablishOutgoingConnection()
+    public function testHandleSocks4ConnectionWithIpv4WillEstablishOutgoingConnection()
     {
         $connection = $this->getMockBuilder('React\Socket\Connection')->disableOriginalConstructor()->setMethods(array('pause', 'end'))->getMock();
 
@@ -165,6 +165,80 @@ public function testHandleSocksConnectionWillEstablishOutgoingConnection()
         $connection->emit('data', array("\x04\x01" . "\x00\x50" . pack('N', ip2long('127.0.0.1')) . "\x00"));
     }
 
+    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();
+
+        $promise = new Promise(function () { });
+
+        $this->connector->expects($this->once())->method('connect')->with('127.0.0.1:80')->willReturn($promise);
+
+        $this->server->onConnection($connection);
+
+        $connection->emit('data', array("\x05\x01\x00" . "\x05\x01\x00\x01" . pack('N', ip2long('127.0.0.1')) . "\x00\x50"));
+    }
+
+    public function testHandleSocks5ConnectionWithIpv6WillEstablishOutgoingConnection()
+    {
+        $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('[::1]:80')->willReturn($promise);
+
+        $this->server->onConnection($connection);
+
+        $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();