Bug 755412 - Debugging protocol server should drop connection if packet framing is bad. r=past, r=jimb
authorMike Hordecki <mhordecki@mozilla.com>
Fri, 16 Aug 2013 08:48:22 -0400
changeset 143001 7e8931d11672c944f96695a866cd9edbceadc3bb
parent 143000 f9d39959fc55d35e54047eedf86eb74d2fea0c46
child 143002 4868b76a3aa9bd232a90fcca3c63c2a495a5cd31
push id25119
push userphilringnalda@gmail.com
push dateMon, 19 Aug 2013 00:45:03 +0000
treeherdermozilla-central@c8c9bd74cc40 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspast, jimb
bugs755412
milestone26.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 755412 - Debugging protocol server should drop connection if packet framing is bad. r=past, r=jimb
toolkit/devtools/server/tests/unit/test_dbgsocket_connection_drop.js
toolkit/devtools/server/tests/unit/xpcshell.ini
toolkit/devtools/server/transport.js
new file mode 100644
--- /dev/null
+++ b/toolkit/devtools/server/tests/unit/test_dbgsocket_connection_drop.js
@@ -0,0 +1,73 @@
+/**
+ * Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/
+ */
+
+/**
+ * Bug 755412 - checks if the server drops the connection on an improperly
+ * framed packet, i.e. when the length header is invalid.
+ */
+
+Cu.import("resource://gre/modules/devtools/dbg-server.jsm");
+Cu.import("resource://gre/modules/devtools/dbg-client.jsm");
+Cu.import("resource://gre/modules/devtools/DevToolsUtils.jsm");
+
+let port = 2929;
+
+function run_test()
+{
+  do_print("Starting test at " + new Date().toTimeString());
+  initTestDebuggerServer();
+
+  add_test(test_socket_conn_drops_after_invalid_header);
+  add_test(test_socket_conn_drops_after_invalid_header_2);
+  add_test(test_socket_conn_drops_after_too_long_header);
+  run_next_test();
+}
+
+function test_socket_conn_drops_after_invalid_header() {
+  return test_helper('fluff30:27:{"to":"root","type":"echo"}');
+}
+
+function test_socket_conn_drops_after_invalid_header_2() {
+  return test_helper('27asd:{"to":"root","type":"echo"}');
+}
+
+function test_socket_conn_drops_after_too_long_header() {
+  return test_helper('4305724038957487634549823475894325');
+}
+
+
+function test_helper(payload) {
+  try_open_listener();
+
+  let transport = debuggerSocketConnect("127.0.0.1", port);
+  transport.hooks = {
+    onPacket: function(aPacket) {
+      this.onPacket = function(aPacket) {
+        do_throw(new Error("This connection should be dropped."));
+        transport.close();
+      }
+
+      // Inject the payload directly into the stream.
+      transport._outgoing += payload;
+      transport._flushOutgoing();
+    },
+    onClosed: function(aStatus) {
+      do_check_true(true);
+      run_next_test();
+    },
+  };
+  transport.ready();
+}
+
+function try_open_listener()
+{
+  try {
+    do_check_true(DebuggerServer.openListener(port));
+  } catch (e) {
+    // In case the port is unavailable, pick a random one between 2000 and 65000.
+    port = Math.floor(Math.random() * (65000 - 2000 + 1)) + 2000;
+    try_open_listener();
+  }
+}
--- a/toolkit/devtools/server/tests/unit/xpcshell.ini
+++ b/toolkit/devtools/server/tests/unit/xpcshell.ini
@@ -3,16 +3,17 @@ head = head_dbg.js
 tail =
 
 [test_forwardingprefix.js]
 [test_getyoungestframe.js]
 [test_nsjsinspector.js]
 [test_dbgsocket.js]
 skip-if = toolkit == "gonk"
 reason = bug 821285
+[test_dbgsocket_connection_drop.js]
 [test_dbgactor.js]
 [test_dbgglobal.js]
 [test_dbgclient_debuggerstatement.js]
 [test_attach.js]
 [test_blackboxing-01.js]
 [test_blackboxing-02.js]
 [test_blackboxing-03.js]
 [test_blackboxing-04.js]
--- a/toolkit/devtools/server/transport.js
+++ b/toolkit/devtools/server/transport.js
@@ -146,20 +146,32 @@ DebuggerTransport.prototype = {
    * not contain a full packet yet. After a proper packet is parsed, the dispatch
    * handler DebuggerTransport.hooks.onPacket is called with the packet as a
    * parameter.
    */
   _processIncoming: function DT__processIncoming() {
     // Well this is ugly.
     let sep = this._incoming.indexOf(':');
     if (sep < 0) {
+      // Incoming packet length is too big anyway - drop the connection.
+      if (this._incoming.length > 20) {
+        this.close();
+      }
+
       return false;
     }
 
-    let count = parseInt(this._incoming.substring(0, sep));
+    let count = this._incoming.substring(0, sep);
+    // Check for a positive number with no garbage afterwards.
+    if (!/^[0-9]+$/.exec(count)) {
+      this.close();
+      return false;
+    }
+
+    count = +count;
     if (this._incoming.length - (sep + 1) < count) {
       // Don't have a complete request yet.
       return false;
     }
 
     // We have a complete request, pluck it out of the data and parse it.
     this._incoming = this._incoming.substring(sep + 1);
     let packet = this._incoming.substring(0, count);