Merge branch 'JsMqtt-5-Validation' into JsMqtt-6-Protocol

Author: Bret Ambrose

continuous-delivery/build-manylinux2014-aarch64.sh

@@ -39,7 +39,7 @@ mkdir $UNZIP
 tar -xf aws-crt-$CURRENT_TAG.tgz -C $UNZIP
 PACK_FILE_SIZE_KB=$(du -sk $UNZIP | awk '{print $1}')
 echo "Current package size: ${PACK_FILE_SIZE_KB}"
-if expr $PACK_FILE_SIZE_KB \> "$((35000))" ; then
+if expr $PACK_FILE_SIZE_KB \> "$((37000))" ; then
     # the package size is too large, return -1
     echo "Package size is too large!"
     exit -1

continuous-delivery/pack.sh

@@ -10,7 +10,8 @@
         "../lib/browser/mqtt5.ts",
         "../lib/browser/ws.ts",
         "../lib/common/event.ts",
-        "../lib/common/http.ts"
+        "../lib/common/http.ts",
+        "../lib/common/mqtt_shared.ts"
     ],
     "tsconfig": "../tsconfig.browser.json",
     "out": "../docs/browser",

crt/aws-c-mqtt

@@ -14,7 +14,8 @@
         "../lib/common/event.ts",
         "../lib/common/http.ts",
         "../lib/common/platform.ts",
-        "../lib/common/resource_safety.ts"
+        "../lib/common/resource_safety.ts",
+        "../lib/common/mqtt_shared.ts"
     ],
     "out": "../docs/node",
     "excludeExternals": true,

docsrc/typedoc-browser.json

@@ -47,16 +47,12 @@ export function decodeLengthPrefixedString(payload: DataView, offset: number) :
     return [toUtf8(new Uint8Array(payload.buffer, index, stringLength)), index + stringLength];
 }
 
-//Buffer.from(arrayBuffer, view.byteOffset, view.byteLength)
-
 export function decodeBytes(payload: DataView, offset: number, length: number) : [ArrayBufferView, number] {
-    // return [payload.buffer.slice(offset, offset + length), offset + length];
     return [Buffer.from(payload.buffer, offset, length), offset + length];
 }
 
 export function decodeLengthPrefixedBytes(payload: DataView, offset: number) : [ArrayBufferView, number] {
     let [bytesLength, index] = decodeU16(payload, offset);
-    // return [payload.buffer.slice(index, index + bytesLength), index + bytesLength];
     return [Buffer.from(payload.buffer, index, bytesLength), index + bytesLength];
 }
 
@@ -77,6 +73,10 @@ export function decodeUserProperty(payload: DataView, offset: number, userProper
 // MQTT 311 Packet decoding functions
 
 function decodeConnackPacket311(firstByte: number, payload: DataView) : model.ConnackPacketInternal {
+    if (firstByte != model.PACKET_TYPE_FIRST_BYTE_CONNACK) {
+        throw new CrtError("Connack with invalid first byte: " + firstByte);
+    }
+
     if (payload.byteLength != 2) {
         throw new CrtError("Connack packet invalid payload length");
     }
@@ -310,6 +310,10 @@ function decodeConnackPacket5(firstByte: number, payload: DataView) : model.Conn
     };
 
     [flags, index] = decodeU8(payload, index);
+    if ((flags & (~0x01)) != 0) {
+        throw new CrtError("Connack invalid flags");
+    }
+
     connack.sessionPresent = (flags & model.CONNACK_FLAGS_SESSION_PRESENT) != 0;
     [connack.reasonCode, index] = decodeU8(payload, index);
 

docsrc/typedoc-node.json

@@ -355,7 +355,7 @@ function getConnectPacketRemainingLengths5(packet: model.ConnectPacketBinary) :
     }
 
     if (packet.will) {
-        remaining_length += vli.getVliByteLength(will_properties_length) + will_properties_length
+        remaining_length += vli.getVliByteLength(will_properties_length) + will_properties_length;
         remaining_length += 2 + packet.will.topicName.byteLength;
         remaining_length += 2; // payload length
         if (packet.will.payload) {

lib/browser/mqtt_internal/decoder.ts

@@ -1241,8 +1241,8 @@ test('Binary connect packet validation - will too long', async () => {
     // @ts-ignore
     packet.will.payload = new Uint8Array(65537);
 
-    expect(() => { validate.validateBinaryOutboundPacket(packet, model.ProtocolMode.Mqtt311, settings); }).toThrow("exceeds established maximum packet size");
-    expect(() => { validate.validateBinaryOutboundPacket(packet, model.ProtocolMode.Mqtt5, settings); }).toThrow("exceeds established maximum packet size");
+    expect(() => { validate.validateBinaryOutboundPacket(packet, model.ProtocolMode.Mqtt311, settings); }).toThrow("will payload too long");
+    expect(() => { validate.validateBinaryOutboundPacket(packet, model.ProtocolMode.Mqtt5, settings); }).toThrow("will payload too long");
 });
 
 // Disconnect Validation

lib/browser/mqtt_internal/encoder.ts

@@ -243,7 +243,7 @@ function validateSubackReasonCode(value: mqtt5_packet.SubackReasonCode, mode: mo
     }
 }
 
-function validatePubackReasonCode(value: mqtt5_packet.PubackReasonCode, mode: model.ProtocolMode) {
+function validatePubackReasonCode(value: mqtt5_packet.PubackReasonCode) {
     if (mqtt5_packet.PubackReasonCode[value] === undefined) {
         throw new CrtError(`"${value}" is not a valid MQTT5 PubackReasonCode`);
     }
@@ -510,7 +510,11 @@ function validateBinaryUserProperties(userProperties: Array<model.UserPropertyBi
 
 function validateBinaryPublish(packet: model.PublishPacketBinary, mode: model.ProtocolMode, settings: mqtt5_common.NegotiatedSettings, isWill: boolean) {
     if (isWill) {
-        validatePacketLength(packet, mode, 65535);
+        // topic will be checked below
+
+        if (packet.payload && packet.payload.byteLength > 65535) {
+            throw new CrtError("will payload too long");
+        }
     } else {
         validatePacketLength(packet, mode, settings.maximumPacketSizeToServer);
     }
@@ -676,7 +680,7 @@ function validateInboundPublish(packet: model.PublishPacketInternal, mode: model
 
 function validateInboundPuback(packet: model.PubackPacketInternal, mode: model.ProtocolMode) {
     validateRequiredPacketId(packet.packetId, "packetId");
-    validatePubackReasonCode(packet.reasonCode, mode);
+    validatePubackReasonCode(packet.reasonCode);
 }
 
 function validateInboundConnack(packet: model.ConnackPacketInternal, mode: model.ProtocolMode) {

lib/browser/mqtt_internal/validate.spec.ts

@@ -160,6 +160,35 @@ test('VLI encoding overflow', () => {
     expect(() => { vli.encodeVli(view1, 128 * 128 * 128 * 128) }).toThrow("Invalid VLI value");
 });
 
+test('VLI decoding invalid offset', () => {
+    let buffer1 = new Uint8Array([0, 128, 0]);
+    expect(() => { vli.decodeVli(new DataView(buffer1.buffer, 1, 1), -1) }).toThrow("Invalid DataView offset");
+});
+
+test('VLI decoding - insufficient data', () => {
+    let buffer0 = new Uint8Array([0]);
+    expect(vli.decodeVli(new DataView(buffer0.buffer, 0, 0), 0).type).toBe(vli.VliDecodeResultType.MoreData);
+
+    let buffer1 = new Uint8Array([0, 128, 0]);
+    expect(vli.decodeVli(new DataView(buffer1.buffer, 1, 0), 0).type).toBe(vli.VliDecodeResultType.MoreData);
+    expect(vli.decodeVli(new DataView(buffer1.buffer, 1, 1), 0).type).toBe(vli.VliDecodeResultType.MoreData);
+
+    let buffer2 = new Uint8Array([0, 0, 128, 128, 0]);
+    expect(vli.decodeVli(new DataView(buffer2.buffer, 2, 0), 0).type).toBe(vli.VliDecodeResultType.MoreData);
+    expect(vli.decodeVli(new DataView(buffer2.buffer, 2, 1), 0).type).toBe(vli.VliDecodeResultType.MoreData);
+    expect(vli.decodeVli(new DataView(buffer2.buffer, 2, 2), 0).type).toBe(vli.VliDecodeResultType.MoreData);
+
+    let buffer3 = new Uint8Array([0, 0, 0, 128, 128, 128, 0]);
+    expect(vli.decodeVli(new DataView(buffer3.buffer, 3, 0), 0).type).toBe(vli.VliDecodeResultType.MoreData);
+    expect(vli.decodeVli(new DataView(buffer3.buffer, 3, 1), 0).type).toBe(vli.VliDecodeResultType.MoreData);
+    expect(vli.decodeVli(new DataView(buffer3.buffer, 3, 2), 0).type).toBe(vli.VliDecodeResultType.MoreData);
+    expect(vli.decodeVli(new DataView(buffer3.buffer, 3, 3), 0).type).toBe(vli.VliDecodeResultType.MoreData);
+
+    expect(vli.decodeVli(new DataView(buffer3.buffer, 1, 2), 2).type).toBe(vli.VliDecodeResultType.MoreData);
+    expect(vli.decodeVli(new DataView(buffer3.buffer, 1, 3), 2).type).toBe(vli.VliDecodeResultType.MoreData);
+    expect(vli.decodeVli(new DataView(buffer3.buffer, 1, 4), 2).type).toBe(vli.VliDecodeResultType.MoreData);
+    expect(vli.decodeVli(new DataView(buffer3.buffer, 1, 5), 2).type).toBe(vli.VliDecodeResultType.MoreData);
+});
 
 test('VLI decoding - 1 byte', () => {
     let buffer_0 = new Uint8Array([0]);
@@ -173,6 +202,11 @@ test('VLI decoding - 1 byte', () => {
 
     let buffer_127 = new Uint8Array([127]);
     expect(vli.decodeVli(new DataView(buffer_127.buffer), 0).value).toBe(127);
+
+    let buffer_127_offset = new Uint8Array([0, 127, 0]);
+    let result = vli.decodeVli(new DataView(buffer_127_offset.buffer), 1);
+    expect(result.value).toBe(127);
+    expect(result.nextOffset).toBe(2);
 });
 
 test('VLI decoding - 2 byte', () => {
@@ -193,6 +227,11 @@ test('VLI decoding - 2 byte', () => {
 
     let buffer_16382 = new Uint8Array([254, 127]);
     expect(vli.decodeVli(new DataView(buffer_16382.buffer), 0).value).toBe(16382);
+
+    let buffer_16382_offset = new Uint8Array([0, 0, 254, 127, 0]);
+    let result = vli.decodeVli(new DataView(buffer_16382_offset.buffer), 2);
+    expect(result.value).toBe(16382);
+    expect(result.nextOffset).toBe(4);
 });
 
 test('VLI decoding - 3 byte', () => {
@@ -210,6 +249,11 @@ test('VLI decoding - 3 byte', () => {
 
     let buffer5 = new Uint8Array([255, 255, 127]);
     expect(vli.decodeVli(new DataView(buffer5.buffer), 0).value).toBe(128 * 128 * 128 - 1);
+
+    let buffer5_offset = new Uint8Array([0, 0, 0, 255, 255, 127, 0]);
+    let result = vli.decodeVli(new DataView(buffer5_offset.buffer), 3);
+    expect(result.value).toBe(128 * 128 * 128 - 1);
+    expect(result.nextOffset).toBe(6);
 });
 
 test('VLI decoding - 4 byte', () => {
@@ -219,8 +263,13 @@ test('VLI decoding - 4 byte', () => {
     let buffer2 = new Uint8Array([129, 128, 128, 1]);
     expect(vli.decodeVli(new DataView(buffer2.buffer), 0).value).toBe(128 * 128 * 128 + 1);
 
-    let buffer5 = new Uint8Array([255, 255, 255, 127]);
-    expect(vli.decodeVli(new DataView(buffer5.buffer), 0).value).toBe(128 * 128 * 128 * 128 - 1);
+    let buffer3 = new Uint8Array([255, 255, 255, 127]);
+    expect(vli.decodeVli(new DataView(buffer3.buffer), 0).value).toBe(128 * 128 * 128 * 128 - 1);
+
+    let buffer3_offset = new Uint8Array([0, 0, 0, 0, 255, 255, 255, 127, 0]);
+    let result = vli.decodeVli(new DataView(buffer3_offset.buffer), 4);
+    expect(result.value).toBe(128 * 128 * 128 * 128 - 1);
+    expect(result.nextOffset).toBe(8);
 });
 
 function doRoundTripEncodeDecodeVliTest(value: number) {