Add check for 0 packet ID for subacks (FreeRTOS#159)

* Add check for 0 packet ID for subacks * Sort lexicon.txt
muneebahmed10 · Jul 8, 2021 · d39236e · d39236e
1 parent a7818cb
commit d39236e
Show file tree

Hide file tree

Showing 5 changed files with 22 additions and 6 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,7 +3,8 @@
 ## Commits to `main`
 
 ### Updates
- - [#163](https://github.com/FreeRTOS/coreMQTT/pull/163) Fix bug to check for ping responses within `MQTT_PINGRESP_TIMEOUT_MS` instead of the entire keep alive interval
+ - [#163](https://github.com/FreeRTOS/coreMQTT/pull/163) Fix bug to check for ping responses within `MQTT_PINGRESP_TIMEOUT_MS` instead of the entire keep alive interval.
+ - [#159](https://github.com/FreeRTOS/coreMQTT/pull/159) Add more checks for malformed packets when deserializing acknowledgments.
 
 ## v1.1.1 (February 2021)
 

diff --git a/LICENSE b/LICENSE
diff --git a/lexicon.txt b/lexicon.txt
@@ -32,7 +32,6 @@ cleansession
 clientidentifierlength
 cmock
 colspan
-copydoc
 com
 cond
 config
@@ -42,6 +41,7 @@ connack
 connectinfo
 connectpacketsize
 const
+copydoc
 coremqtt
 csdk
 css
@@ -52,11 +52,11 @@ defragmenting
 deserialization
 deserializationresult
 deserialize
-deserializers
 deserializeack
 deserialized
 deserializepublish
 deserializer
+deserializers
 deserializestatus
 deserializing
 didn
@@ -126,10 +126,10 @@ logwarn
 lsb
 lwt
 mainpage
-mdash
 malloc
 managekeepalive
 matchtopic
+mdash
 memcpy
 memset
 metadata

diff --git a/source/core_mqtt_serializer.c b/source/core_mqtt_serializer.c
@@ -1164,8 +1164,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 == 0U )
+        {
+            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;