CVE-2017-10910: Uncontrolled Recursion (CWE-674)
value: MQTT.js 2.x.x prior to 2.15.0 issue in handling PUBLISH tickets may lead to an attacker causing a denial-of-service condition.
CWE description: The product does not properly control the amount of recursion that takes place,  consuming excessive resources, such as allocated memory or the program stack.
CVSS v3 base score: 6.5
Published: 2017-12-28T02:29Z
Repository: https://github.com/mqttjs/MQTT.js
Language: JavaScript
Files changed: lib/client.js, test/client.js
Commit date: 2017-12-09 20:00:42 +0100
Commit message: Do not stack overflow if a TCP frame contains too many PUBLISH

Diff: diff --git a/lib/client.js b/lib/client.js
index 7c0a846..41cb2c4 100644
--- a/lib/client.js
+++ b/lib/client.js
@@ -247,26 +247,30 @@ MqttClient.prototype._setupStream = function () {
 
   parser.on('packet', function (packet) {
     packets.push(packet)
   })
 
-  function process () {
+  function nextTickWork () {
+    process.nextTick(work)
+  }
+
+  function work () {
     var packet = packets.shift()
     var done = completeParse
 
     if (packet) {
-      that._handlePacket(packet, process)
+      that._handlePacket(packet, nextTickWork)
     } else {
       completeParse = null
       done()
     }
   }
 
   writable._write = function (buf, enc, done) {
     completeParse = done
     parser.parse(buf)
-    process()
+    work()
   }
 
   this.stream.pipe(writable)
 
   // Suppress connection errors
diff --git a/test/client.js b/test/client.js
index 6da92f4..7dc3301 100644
--- a/test/client.js
+++ b/test/client.js
@@ -5,10 +5,13 @@ var should = require('should')
 var fork = require('child_process').fork
 var path = require('path')
 var abstractClientTests = require('./abstract_client')
 var net = require('net')
 var eos = require('end-of-stream')
+var mqttPacket = require('mqtt-packet')
+var Buffer = require('safe-buffer').Buffer
+var Duplex = require('readable-stream').Duplex
 var Connection = require('mqtt-connection')
 var Server = require('./server')
 var port = 9876
 var server
 
@@ -146,10 +149,58 @@ describe('MqttClient', function () {
             done()
           }
         })
       })
     })
+
+    it('should not go overflow if the TCP frame contains a lot of PUBLISH packets', function (done) {
+      var parser = mqttPacket.parser()
+      var count = 0
+      var max = 1000
+      var duplex = new Duplex({
+        read: function (n) {},
+        write: function (chunk, enc, cb) {
+          parser.parse(chunk)
+          cb() // nothing to do
+        }
+      })
+      var client = new mqtt.MqttClient(function () {
+        return duplex
+      }, {})
+
+      client.on('message', function (t, p, packet) {
+        if (++count === max) {
+          done()
+        }
+      })
+
+      parser.on('packet', function (packet) {
+        var packets = []
+
+        if (packet.cmd === 'connect') {
+          duplex.push(mqttPacket.generate({
+            cmd: 'connack',
+            sessionPresent: false,
+            returnCode: 0
+          }))
+
+          for (var i = 0; i < max; i++) {
+            packets.push(mqttPacket.generate({
+              cmd: 'publish',
+              topic: Buffer.from('hello'),
+              payload: Buffer.from('world'),
+              retain: false,
+              dup: false,
+              messageId: i + 1,
+              qos: 1
+            }))
+          }
+
+          duplex.push(Buffer.concat(packets))
+        }
+      })
+    })
   })
 
   describe('reconnecting', function () {
     it('should attempt to reconnect once server is down', function (done) {
       this.timeout(15000)

Vulnerable code:   function process () {
      that._handlePacket(packet, process)
    process()
Fixed code:   function nextTickWork () {
    process.nextTick(work)
  }

  function work () {
      that._handlePacket(packet, nextTickWork)
    work()
var mqttPacket = require('mqtt-packet')
var Buffer = require('safe-buffer').Buffer
var Duplex = require('readable-stream').Duplex

    it('should not go overflow if the TCP frame contains a lot of PUBLISH packets', function (done) {
      var parser = mqttPacket.parser()
      var count = 0
      var max = 1000
      var duplex = new Duplex({
        read: function (n) {},
        write: function (chunk, enc, cb) {
          parser.parse(chunk)
          cb() // nothing to do
        }
      })
      var client = new mqtt.MqttClient(function () {
        return duplex
      }, {})

      client.on('message', function (t, p, packet) {
        if (++count === max) {
          done()
        }
      })

      parser.on('packet', function (packet) {
        var packets = []

        if (packet.cmd === 'connect') {
          duplex.push(mqttPacket.generate({
            cmd: 'connack',
            sessionPresent: false,
            returnCode: 0
          }))

          for (var i = 0; i < max; i++) {
            packets.push(mqttPacket.generate({
              cmd: 'publish',
              topic: Buffer.from('hello'),
              payload: Buffer.from('world'),
              retain: false,
              dup: false,
              messageId: i + 1,
              qos: 1
            }))
          }

          duplex.push(Buffer.concat(packets))
        }
      })
    })