Add check for 0 packet ID for subacks

FreeRTOS · Apr 1, 2021 · e5656b4 · e5656b4
1 parent f756d98
commit e5656b4
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 2 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,10 @@
 # Changelog for coreMQTT Client Library
 
+## Commits to `main`
+
+### Changes
+ - Add more checks for malformed packets when deserializing acknowledgments.
+
 ## v1.1.1 (February 2021)
 
 ### Changes

diff --git a/LICENSE b/LICENSE
diff --git a/source/core_mqtt_serializer.c b/source/core_mqtt_serializer.c
@@ -24,9 +24,11 @@
  * @file core_mqtt_serializer.c
  * @brief Implements the user-facing functions in core_mqtt_serializer.h.
  */
+#include <cstdint>
 #include <string.h>
 #include <assert.h>
 
+#include "core_mqtt.h"
 #include "core_mqtt_serializer.h"
 
 /**
@@ -1164,8 +1166,15 @@ static MQTTStatus_t deserializeSuback( const MQTTPacketInfo_t * pSuback,
         LogDebug( ( "Packet identifier %hu.",
                     ( unsigned short ) *pPacketIdentifier ) );
 
-        status = readSubackStatus( remainingLength - sizeof( uint16_t ),
-                                   pVariableHeader + sizeof( uint16_t ) );
+        if( *pPacketIdentifier == MQTT_PACKET_ID_INVALID )
+        {
+            status = MQTTBadResponse;
+        }
+        else
+        {
+            status = readSubackStatus( remainingLength - sizeof( uint16_t ),
+                                       pVariableHeader + sizeof( uint16_t ) );
+        }
     }
 
     return status;

diff --git a/test/unit-test/core_mqtt_serializer_utest.c b/test/unit-test/core_mqtt_serializer_utest.c
@@ -1480,6 +1480,14 @@ void test_MQTT_DeserializeAck_suback( void )
     status = MQTT_DeserializeAck( &mqttPacketInfo, &packetIdentifier, &sessionPresent );
     TEST_ASSERT_EQUAL_INT( MQTTBadResponse, status );
 
+    /* Invalid packet ID. */
+    buffer[ 0 ] = 0;
+    buffer[ 1 ] = 0;
+    mqttPacketInfo.remainingLength = 3;
+    buffer[ 2 ] = 0;
+    status = MQTT_DeserializeAck( &mqttPacketInfo, &packetIdentifier, &sessionPresent );
+    TEST_ASSERT_EQUAL_INT( MQTTBadResponse, status );
+
     /* Set packet identifier. */
     buffer[ 0 ] = 0;
     buffer[ 1 ] = 1;