From 810429a6fe48285583ee02e9eb0731b7937e55ec Mon Sep 17 00:00:00 2001 From: Chris Boden Date: Sun, 1 Oct 2017 11:11:38 -0400 Subject: [PATCH 1/3] Re-encode topic from WAMP msg to safeguard The client is supposed to send a string as a topic but we're goign to re-json'ify the topic in case of bad client input. fixes #516 --- src/Ratchet/Wamp/ServerProtocol.php | 14 ++++-- tests/unit/Wamp/ServerProtocolTest.php | 67 +++++++++++++++++--------- 2 files changed, 52 insertions(+), 29 deletions(-) diff --git a/src/Ratchet/Wamp/ServerProtocol.php b/src/Ratchet/Wamp/ServerProtocol.php index d892409..db0e150 100644 --- a/src/Ratchet/Wamp/ServerProtocol.php +++ b/src/Ratchet/Wamp/ServerProtocol.php @@ -62,9 +62,9 @@ class ServerProtocol implements MessageComponentInterface, WsServerInterface { $subs[] = 'wamp'; return $subs; - } else { - return array('wamp'); } + + return ['wamp']; } /** @@ -93,6 +93,10 @@ class ServerProtocol implements MessageComponentInterface, WsServerInterface { throw new Exception("Invalid WAMP message format"); } + if (isset($json[1]) && !is_string($json[1])) { + $json[1] = json_encode($json[1]); + } + switch ($json[0]) { case static::MSG_PREFIX: $from->WAMP->prefixes[$json[1]] = $json[2]; @@ -122,13 +126,13 @@ class ServerProtocol implements MessageComponentInterface, WsServerInterface { $exclude = (array_key_exists(3, $json) ? $json[3] : null); if (!is_array($exclude)) { if (true === (boolean)$exclude) { - $exclude = array($from->WAMP->sessionId); + $exclude = [$from->WAMP->sessionId]; } else { - $exclude = array(); + $exclude = []; } } - $eligible = (array_key_exists(4, $json) ? $json[4] : array()); + $eligible = (array_key_exists(4, $json) ? $json[4] : []); $this->_decorating->onPublish($from, $from->getUri($json[1]), $json[2], $exclude, $eligible); break; diff --git a/tests/unit/Wamp/ServerProtocolTest.php b/tests/unit/Wamp/ServerProtocolTest.php index 082a3f5..e3c6563 100644 --- a/tests/unit/Wamp/ServerProtocolTest.php +++ b/tests/unit/Wamp/ServerProtocolTest.php @@ -4,9 +4,9 @@ use Ratchet\Mock\Connection; use Ratchet\Mock\WampComponent as TestComponent; /** - * @covers Ratchet\Wamp\ServerProtocol - * @covers Ratchet\Wamp\WampServerInterface - * @covers Ratchet\Wamp\WampConnection + * @covers \Ratchet\Wamp\ServerProtocol + * @covers \Ratchet\Wamp\WampServerInterface + * @covers \Ratchet\Wamp\WampConnection */ class ServerProtocolTest extends \PHPUnit_Framework_TestCase { protected $_comp; @@ -23,13 +23,13 @@ class ServerProtocolTest extends \PHPUnit_Framework_TestCase { } public function invalidMessageProvider() { - return array( - array(0) - , array(3) - , array(4) - , array(8) - , array(9) - ); + return [ + [0] + , [3] + , [4] + , [8] + , [9] + ]; } /** @@ -40,7 +40,7 @@ class ServerProtocolTest extends \PHPUnit_Framework_TestCase { $conn = $this->newConn(); $this->_comp->onOpen($conn); - $this->_comp->onMessage($conn, json_encode(array($type))); + $this->_comp->onMessage($conn, json_encode([$type])); } public function testWelcomeMessage() { @@ -82,16 +82,16 @@ class ServerProtocolTest extends \PHPUnit_Framework_TestCase { } public function callProvider() { - return array( - array(2, 'a', 'b') - , array(2, array('a', 'b')) - , array(1, 'one') - , array(3, 'one', 'two', 'three') - , array(3, array('un', 'deux', 'trois')) - , array(2, 'hi', array('hello', 'world')) - , array(2, array('hello', 'world'), 'hi') - , array(2, array('hello' => 'world', 'herp' => 'derp')) - ); + return [ + [2, 'a', 'b'] + , [2, ['a', 'b']] + , [1, 'one'] + , [3, 'one', 'two', 'three'] + , [3, ['un', 'deux', 'trois']] + , [2, 'hi', ['hello', 'world']] + , [2, ['hello', 'world'], 'hi'] + , [2, ['hello' => 'world', 'herp' => 'derp']] + ]; } /** @@ -102,7 +102,7 @@ class ServerProtocolTest extends \PHPUnit_Framework_TestCase { $paramNum = array_shift($args); $uri = 'http://example.com/endpoint/' . rand(1, 100); - $id = uniqid(); + $id = uniqid('', false); $clientMessage = array_merge(array(2, $id, $uri), $args); $conn = $this->newConn(); @@ -145,8 +145,8 @@ class ServerProtocolTest extends \PHPUnit_Framework_TestCase { public function testPublishAndEligible() { $conn = $this->newConn(); - $buddy = uniqid(); - $friend = uniqid(); + $buddy = uniqid('', false); + $friend = uniqid('', false); $this->_comp->onOpen($conn); $this->_comp->onMessage($conn, json_encode(array(7, 'topic', 'event', false, array($buddy, $friend)))); @@ -265,4 +265,23 @@ class ServerProtocolTest extends \PHPUnit_Framework_TestCase { $this->_comp->onOpen($conn); $this->_comp->onMessage($conn, $message); } + + public function testBadClientInputFromNonStringTopic() { + $conn = new WampConnection($this->newConn()); + $this->_comp->onOpen($conn); + + $this->_comp->onMessage($conn, json_encode([5, ['hells', 'nope']])); + + $this->assertEquals('["hells","nope"]', $this->_app->last['onSubscribe'][1]); + } + + public function testBadPrefixWithNonStringTopic() { + $conn = new WampConnection($this->newConn()); + $this->_comp->onOpen($conn); + + $this->_comp->onMessage($conn, json_encode([1, ['hells', 'nope'], ['bad', 'input']])); + $this->_comp->onMessage($conn, json_encode([7, ['bad', 'input'], 'Hider'])); + + $this->assertEquals('["bad","input"]', $this->_app->last['onPublish'][1]); + } } From 309564cbf11d04d66f01cc02a56180416c305514 Mon Sep 17 00:00:00 2001 From: Chris Boden Date: Sat, 14 Oct 2017 21:19:07 -0400 Subject: [PATCH 2/3] Close connection if invalid topic --- src/Ratchet/Wamp/ServerProtocol.php | 2 +- tests/unit/Wamp/ServerProtocolTest.php | 16 ++++++++++++---- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/Ratchet/Wamp/ServerProtocol.php b/src/Ratchet/Wamp/ServerProtocol.php index db0e150..97fa868 100644 --- a/src/Ratchet/Wamp/ServerProtocol.php +++ b/src/Ratchet/Wamp/ServerProtocol.php @@ -94,7 +94,7 @@ class ServerProtocol implements MessageComponentInterface, WsServerInterface { } if (isset($json[1]) && !is_string($json[1])) { - $json[1] = json_encode($json[1]); + throw new Exception('Invalid Topic, must be a string'); } switch ($json[0]) { diff --git a/tests/unit/Wamp/ServerProtocolTest.php b/tests/unit/Wamp/ServerProtocolTest.php index e3c6563..8ff68c2 100644 --- a/tests/unit/Wamp/ServerProtocolTest.php +++ b/tests/unit/Wamp/ServerProtocolTest.php @@ -267,21 +267,29 @@ class ServerProtocolTest extends \PHPUnit_Framework_TestCase { } public function testBadClientInputFromNonStringTopic() { + $this->setExpectedException('\Ratchet\Wamp\Exception'); + $conn = new WampConnection($this->newConn()); $this->_comp->onOpen($conn); $this->_comp->onMessage($conn, json_encode([5, ['hells', 'nope']])); - - $this->assertEquals('["hells","nope"]', $this->_app->last['onSubscribe'][1]); } public function testBadPrefixWithNonStringTopic() { + $this->setExpectedException('\Ratchet\Wamp\Exception'); + $conn = new WampConnection($this->newConn()); $this->_comp->onOpen($conn); $this->_comp->onMessage($conn, json_encode([1, ['hells', 'nope'], ['bad', 'input']])); - $this->_comp->onMessage($conn, json_encode([7, ['bad', 'input'], 'Hider'])); + } - $this->assertEquals('["bad","input"]', $this->_app->last['onPublish'][1]); + public function testBadPublishWithNonStringTopic() { + $this->setExpectedException('\Ratchet\Wamp\Exception'); + + $conn = new WampConnection($this->newConn()); + $this->_comp->onOpen($conn); + + $this->_comp->onMessage($conn, json_encode([7, ['bad', 'input'], 'Hider'])); } } From d52de66ab0d8025494d5747a3b4af6672726860b Mon Sep 17 00:00:00 2001 From: Chris Boden Date: Mon, 11 Dec 2017 19:21:52 -0500 Subject: [PATCH 3/3] Allow numeric topics Not part of the spec but to prevent a BC break --- src/Ratchet/Wamp/ServerProtocol.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Ratchet/Wamp/ServerProtocol.php b/src/Ratchet/Wamp/ServerProtocol.php index 97fa868..2d6d799 100644 --- a/src/Ratchet/Wamp/ServerProtocol.php +++ b/src/Ratchet/Wamp/ServerProtocol.php @@ -93,7 +93,7 @@ class ServerProtocol implements MessageComponentInterface, WsServerInterface { throw new Exception("Invalid WAMP message format"); } - if (isset($json[1]) && !is_string($json[1])) { + if (isset($json[1]) && !(is_string($json[1]) || is_numeric($json[1]))) { throw new Exception('Invalid Topic, must be a string'); }