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 142871 7e8931d11672c944f96695a866cd9edbceadc3bb
parent 142870 f9d39959fc55d35e54047eedf86eb74d2fea0c46
child 142872 4868b76a3aa9bd232a90fcca3c63c2a495a5cd31
push id2229
push userryanvm@gmail.com
push dateFri, 16 Aug 2013 12:50:32 +0000
treeherderfx-team@3e3000551e51 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspast, jimb
bugs755412
milestone26.0a1
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);