Merge pull request #64 from clue-labs/fix-hostname

Fix SOCKS5 client receiving destination hostnames
clue · Sep 1, 2017 · 131aebe · 131aebe
2 parents b08dcd1 + 848b32f
commit 131aebe
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 4 deletions.
diff --git a/src/Client.php b/src/Client.php
@@ -349,15 +349,15 @@ private function handleSocks5(ConnectionInterface $stream, $host, $port, StreamR
             if ($data['type'] === 0x01) {
                 // IPv4 address => skip IP and port
                 return $reader->readLength(6);
-            } else if ($data['type'] === 0x03) {
+            } elseif ($data['type'] === 0x03) {
                 // domain name => read domain name length
                 return $reader->readBinary(array(
                     'length' => 'C'
-                ))->then(function ($data) use ($that) {
+                ))->then(function ($data) use ($reader) {
                     // skip domain name and port
-                    return $that->readLength($data['length'] + 2);
+                    return $reader->readLength($data['length'] + 2);
                 });
-            } else if ($data['type'] === 0x04) {
+            } elseif ($data['type'] === 0x04) {
                 // IPv6 address => skip IP and port
                 return $reader->readLength(18);
             } else {

diff --git a/tests/ClientTest.php b/tests/ClientTest.php
@@ -244,6 +244,42 @@ public function testEmitSocks5DataErrorDuringSessionWillRejectConnection()
         $promise->then(null, $this->expectCallableOnceWithExceptionCode(SOCKET_ECONNREFUSED));
     }
 
+    public function testEmitSocks5DataInvalidAddressTypeWillRejectConnection()
+    {
+        $stream = $this->getMockBuilder('React\Socket\Connection')->disableOriginalConstructor()->setMethods(array('write', 'close'))->getMock();
+        $stream->expects($this->once())->method('close');
+
+        $promise = \React\Promise\resolve($stream);
+
+        $this->connector->expects($this->once())->method('connect')->with('127.0.0.1:1080?hostname=google.com')->willReturn($promise);
+
+        $this->client = new Client('socks5://127.0.0.1:1080', $this->connector);
+
+        $promise = $this->client->connect('google.com:80');
+
+        $stream->emit('data', array("\x05\x00" . "\x05\x00\x00\x00"));
+
+        $promise->then(null, $this->expectCallableOnceWithExceptionCode(SOCKET_EBADMSG));
+    }
+
+    public function testEmitSocks5DataHostnameAddressWillResolveConnection()
+    {
+        $stream = $this->getMockBuilder('React\Socket\Connection')->disableOriginalConstructor()->setMethods(array('write', 'close'))->getMock();
+        $stream->expects($this->never())->method('close');
+
+        $promise = \React\Promise\resolve($stream);
+
+        $this->connector->expects($this->once())->method('connect')->with('127.0.0.1:1080?hostname=google.com')->willReturn($promise);
+
+        $this->client = new Client('socks5://127.0.0.1:1080', $this->connector);
+
+        $promise = $this->client->connect('google.com:80');
+
+        $stream->emit('data', array("\x05\x00" . "\x05\x00\x00\x03\x0Agoogle.com\x00\x50"));
+
+        $promise->then($this->expectCallableOnce());
+    }
+
     public function provideConnectionErrors()
     {
         return array(