From d7a901d2894e73da5b5719c6979d7a5f7ddc356f Mon Sep 17 00:00:00 2001 From: Muneeb Ahmed Date: Thu, 1 Apr 2021 11:20:41 -0700 Subject: [PATCH 1/3] Add check for 0 packet ID for subacks --- CHANGELOG.md | 1 + LICENSE | 0 source/core_mqtt_serializer.c | 13 +++++++++++-- test/unit-test/core_mqtt_serializer_utest.c | 8 ++++++++ 4 files changed, 20 insertions(+), 2 deletions(-) mode change 100755 => 100644 LICENSE diff --git a/CHANGELOG.md b/CHANGELOG.md index 99e320bfe..5d9ea3bed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### 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 + - Add more checks for malformed packets when deserializing acknowledgments. ## v1.1.1 (February 2021) diff --git a/LICENSE b/LICENSE old mode 100755 new mode 100644 diff --git a/source/core_mqtt_serializer.c b/source/core_mqtt_serializer.c index 66b74dd0c..7e8e30b45 100644 --- 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 #include #include +#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 index ddef3c5d9..9cdb68e2f 100644 --- 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; From 626f048052a1e5ec90c3c8d1657bb12b8e676e80 Mon Sep 17 00:00:00 2001 From: Muneeb Ahmed Date: Thu, 1 Apr 2021 11:27:27 -0700 Subject: [PATCH 2/3] Fix errors --- CHANGELOG.md | 4 ++-- source/core_mqtt_serializer.c | 4 +--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5d9ea3bed..6dabc35d2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,8 +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 - - Add more checks for malformed packets when deserializing acknowledgments. + - [#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/source/core_mqtt_serializer.c b/source/core_mqtt_serializer.c index 7e8e30b45..ea466d00a 100644 --- a/source/core_mqtt_serializer.c +++ b/source/core_mqtt_serializer.c @@ -24,11 +24,9 @@ * @file core_mqtt_serializer.c * @brief Implements the user-facing functions in core_mqtt_serializer.h. */ -#include #include #include -#include "core_mqtt.h" #include "core_mqtt_serializer.h" /** @@ -1166,7 +1164,7 @@ static MQTTStatus_t deserializeSuback( const MQTTPacketInfo_t * pSuback, LogDebug( ( "Packet identifier %hu.", ( unsigned short ) *pPacketIdentifier ) ); - if( *pPacketIdentifier == MQTT_PACKET_ID_INVALID ) + if( *pPacketIdentifier == 0U ) { status = MQTTBadResponse; } From 6462a8958ce19eb13f8c45aebd9a275472a9edf1 Mon Sep 17 00:00:00 2001 From: Muneeb Ahmed Date: Wed, 7 Jul 2021 13:59:01 -0700 Subject: [PATCH 3/3] Sort lexicon.txt --- lexicon.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lexicon.txt b/lexicon.txt index ca1109453..1908f6d0a 100644 --- 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