Add mandatory payload checks (#121)

* Add mandatory payload checks * Fix logic implementation
mqttjs · Sep 7, 2021 · 5023255 · 5023255
1 parent aae4e2c
commit 5023255
Show file tree

Hide file tree

Showing 2 changed files with 61 additions and 0 deletions.
diff --git a/parser.js b/parser.js
@@ -347,6 +347,8 @@ class Parser extends EventEmitter {
       }
     }
 
+    if (packet.length <= 0) { return this._emitError(new Error('Malformed subscribe, no payload specified')) }
+
     while (this._pos < packet.length) {
       // Parse topic
       topic = this._parseString()
@@ -411,6 +413,8 @@ class Parser extends EventEmitter {
       }
     }
 
+    if (packet.length <= 0) { return this._emitError(new Error('Malformed suback, no payload specified')) }
+
     // Parse granted QoSes
     while (this._pos < this.packet.length) {
       const code = this._list.readUInt8(this._pos++)
@@ -444,6 +448,8 @@ class Parser extends EventEmitter {
       }
     }
 
+    if (packet.length <= 0) { return this._emitError(new Error('Malformed unsubscribe, no payload specified')) }
+
     while (this._pos < packet.length) {
       // Parse topic
       const topic = this._parseString()
@@ -459,6 +465,13 @@ class Parser extends EventEmitter {
     debug('_parseUnsuback')
     const packet = this.packet
     if (!this._parseMessageId()) return this._emitError(new Error('Cannot parse messageId'))
+
+    if ((this.settings.protocolVersion === 3 ||
+      this.settings.protocolVersion === 4) && packet.length !== 2) {
+      return this._emitError(new Error('Malformed unsuback, payload length must be 2'))
+    }
+    if (packet.length <= 0) { return this._emitError(new Error('Malformed unsuback, no payload specified')) }
+
     // Properties mqtt 5
     if (this.settings.protocolVersion === 5) {
       const properties = this._parseProperties()
@@ -467,6 +480,7 @@ class Parser extends EventEmitter {
       }
       // Parse granted QoSes
       packet.granted = []
+
       while (this._pos < this.packet.length) {
         const code = this._list.readUInt8(this._pos++)
         if (!constants.MQTT5_UNSUBACK_CODES[code]) {

diff --git a/test.js b/test.js
@@ -1274,6 +1274,53 @@ testParseError('Will QoS must be set to zero when Will Flag is set to 0', Buffer
   0, 30 // Keepalive
 ]))
 
+// CONNECT, SUBSCRIBE, SUBACK, UNSUBSCRIBE, UNSUBACK (v.5) packets must have payload
+// CONNECT
+testParseError('Packet too short', Buffer.from([
+  16, // Header
+  8, // Packet length
+  0, 4, // Protocol ID length
+  77, 81, 84, 84, // MQTT
+  5, // Version
+  2, // Clean Start enabled
+  0, 0, // Keep-Alive
+  0, // Property Length
+  0, 0 // Properties
+  // No payload
+]), { protocolVersion: 5 })
+// SUBSCRIBE
+testParseError('Malformed subscribe, no payload specified', Buffer.from([
+  130, // Header
+  0 // Packet length
+]), { protocolVersion: 5 })
+// SUBACK
+testParseError('Malformed suback, no payload specified', Buffer.from([
+  144, // Header
+  0 // Packet length
+]), { protocolVersion: 5 })
+// UNSUBSCRIBE
+testParseError('Malformed unsubscribe, no payload specified', Buffer.from([
+  162, // Header
+  0 // Packet length
+]), { protocolVersion: 5 })
+// UNSUBACK (v.5)
+testParseError('Malformed unsuback, no payload specified', Buffer.from([
+  176, // Header
+  0 // Packet length
+]), { protocolVersion: 5 })
+// UNSUBACK (v.4)
+testParseError('Malformed unsuback, payload length must be 2', Buffer.from([
+  176, // Header
+  1, // Packet length
+  1
+]), { protocolVersion: 4 })
+// UNSUBACK (v.3)
+testParseError('Malformed unsuback, payload length must be 2', Buffer.from([
+  176, // Header
+  1, // Packet length
+  1
+]), { protocolVersion: 3 })
+
 testParseGenerate('connack with return code 0', {
   cmd: 'connack',
   retain: false,