From 311e4f284343060856670d680db8e736902533cd Mon Sep 17 00:00:00 2001 From: ryancw <1831931681@qq.com> Date: Sat, 25 Oct 2025 01:26:44 +0800 Subject: [PATCH 1/5] fix: support zero-length clientId --- source/core_mqtt_serializer.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/source/core_mqtt_serializer.c b/source/core_mqtt_serializer.c index 729817629..e19d4ae07 100644 --- a/source/core_mqtt_serializer.c +++ b/source/core_mqtt_serializer.c @@ -1706,9 +1706,14 @@ MQTTStatus_t MQTT_GetConnectPacketSize( const MQTTConnectInfo_t * pConnectInfo, ( void * ) pPacketSize ) ); status = MQTTBadParameter; } - else if( ( pConnectInfo->clientIdentifierLength == 0U ) || ( pConnectInfo->pClientIdentifier == NULL ) ) + else if( ( pConnectInfo->clientIdentifierLength == 0U ) ^ ( ( pConnectInfo->pClientIdentifier == NULL ) || ( *( pConnectInfo->pClientIdentifier ) == '\0' ) ) ) { - LogError( ( "Mqtt_GetConnectPacketSize() client identifier must be set." ) ); + LogError( ( "Client ID length and value mismatch." ) ); + status = MQTTBadParameter; + } + else if( ( pConnectInfo->clientIdentifierLength == 0U ) && ( pConnectInfo->cleanSession == false ) ) + { + LogError( ( "Zero-length client identifier requires cleanSession=true per MQTT 3.1.1." ) ); status = MQTTBadParameter; } else if( ( pWillInfo != NULL ) && ( pWillInfo->payloadLength > ( size_t ) UINT16_MAX ) ) From 1d55cfd2168564be443b4a044099ef002ee07c09 Mon Sep 17 00:00:00 2001 From: Kody Stribrny Date: Fri, 24 Oct 2025 13:10:00 -0700 Subject: [PATCH 2/5] Fix formatting --- source/core_mqtt_serializer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/core_mqtt_serializer.c b/source/core_mqtt_serializer.c index e19d4ae07..f549ced5f 100644 --- a/source/core_mqtt_serializer.c +++ b/source/core_mqtt_serializer.c @@ -1706,7 +1706,7 @@ MQTTStatus_t MQTT_GetConnectPacketSize( const MQTTConnectInfo_t * pConnectInfo, ( void * ) pPacketSize ) ); status = MQTTBadParameter; } - else if( ( pConnectInfo->clientIdentifierLength == 0U ) ^ ( ( pConnectInfo->pClientIdentifier == NULL ) || ( *( pConnectInfo->pClientIdentifier ) == '\0' ) ) ) + else if( ( pConnectInfo->clientIdentifierLength == 0U ) ^ ( ( pConnectInfo->pClientIdentifier == NULL ) || ( *( pConnectInfo->pClientIdentifier ) == '\0' ) ) ) { LogError( ( "Client ID length and value mismatch." ) ); status = MQTTBadParameter; From 37b90bc6b26654a2507edb7c3a78c627446711a1 Mon Sep 17 00:00:00 2001 From: Kody Stribrny Date: Tue, 28 Oct 2025 15:44:34 -0700 Subject: [PATCH 3/5] Add unit tests --- test/unit-test/core_mqtt_serializer_utest.c | 48 +++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/test/unit-test/core_mqtt_serializer_utest.c b/test/unit-test/core_mqtt_serializer_utest.c index c359e296c..8b5df8a25 100644 --- a/test/unit-test/core_mqtt_serializer_utest.c +++ b/test/unit-test/core_mqtt_serializer_utest.c @@ -463,11 +463,59 @@ void test_MQTT_GetConnectPacketSize( void ) status = MQTT_GetConnectPacketSize( &connectInfo, NULL, &remainingLength, &packetSize ); TEST_ASSERT_EQUAL_INT( MQTTBadParameter, status ); + /* Verify NULL client identifier with provided non-zero length fails. */ connectInfo.pClientIdentifier = NULL; connectInfo.clientIdentifierLength = CLIENT_IDENTIFIER_LENGTH; + connectInfo.cleanSession = true; status = MQTT_GetConnectPacketSize( &connectInfo, NULL, &remainingLength, &packetSize ); TEST_ASSERT_EQUAL_INT( MQTTBadParameter, status ); + /* Verify empty string client ID with provided non-zero length fails. */ + connectInfo.pClientIdentifier = ""; + connectInfo.clientIdentifierLength = 99U; + connectInfo.cleanSession = true; + status = MQTT_GetConnectPacketSize( &connectInfo, NULL, &remainingLength, &packetSize ); + TEST_ASSERT_EQUAL_INT( MQTTBadParameter, status ); + + /* Verify zero-length client identifier must have clean session. */ + connectInfo.pClientIdentifier = NULL; + connectInfo.clientIdentifierLength = 0U; + connectInfo.cleanSession = false; + status = MQTT_GetConnectPacketSize( &connectInfo, NULL, &remainingLength, &packetSize ); + TEST_ASSERT_EQUAL_INT( MQTTBadParameter, status ); + + /* Verify zero-length client identifier must have clean session. */ + connectInfo.pClientIdentifier = ""; + connectInfo.clientIdentifierLength = 0U; + connectInfo.cleanSession = false; + status = MQTT_GetConnectPacketSize( &connectInfo, NULL, &remainingLength, &packetSize ); + TEST_ASSERT_EQUAL_INT( MQTTBadParameter, status ); + + /* NULL client ID and client ID length of 0 is treated as zero-length identifier */ + connectInfo.pClientIdentifier = NULL; + connectInfo.clientIdentifierLength = 0U; + connectInfo.cleanSession = true; + connectInfo.pUserName = NULL; + connectInfo.pPassword = NULL; + status = MQTT_GetConnectPacketSize( &connectInfo, NULL, &remainingLength, &packetSize ); + TEST_ASSERT_EQUAL_INT( MQTTSuccess, status ); + /* Make sure remaining size returned is 12. */ + TEST_ASSERT_EQUAL_INT( 12, remainingLength ); + /* Make sure packet size is 14. */ + TEST_ASSERT_EQUAL_INT( 14, packetSize ); + + /* Empty string client ID and client ID length of 0 is treated as zero-length identifier */ + connectInfo.pClientIdentifier = ""; + connectInfo.clientIdentifierLength = 0U; + connectInfo.pUserName = NULL; + connectInfo.pPassword = NULL; + status = MQTT_GetConnectPacketSize( &connectInfo, NULL, &remainingLength, &packetSize ); + TEST_ASSERT_EQUAL_INT( MQTTSuccess, status ); + /* Make sure remaining size returned is 12. */ + TEST_ASSERT_EQUAL_INT( 12, remainingLength ); + /* Make sure packet size is 14. */ + TEST_ASSERT_EQUAL_INT( 14, packetSize ); + /* Test a will message payload length that is too large. */ memset( &connectInfo, 0x0, sizeof( connectInfo ) ); connectInfo.pClientIdentifier = CLIENT_IDENTIFIER; From b7cec15caa460f6249929a737781d08f7eba320f Mon Sep 17 00:00:00 2001 From: Kody Stribrny Date: Wed, 29 Oct 2025 14:14:58 -0700 Subject: [PATCH 4/5] Fix CBMC tests --- .../MQTT_SerializeConnect/MQTT_SerializeConnect_harness.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/cbmc/proofs/MQTT_SerializeConnect/MQTT_SerializeConnect_harness.c b/test/cbmc/proofs/MQTT_SerializeConnect/MQTT_SerializeConnect_harness.c index 15f74cb0b..e61f7afa1 100644 --- a/test/cbmc/proofs/MQTT_SerializeConnect/MQTT_SerializeConnect_harness.c +++ b/test/cbmc/proofs/MQTT_SerializeConnect/MQTT_SerializeConnect_harness.c @@ -39,7 +39,11 @@ void harness() MQTTStatus_t status = MQTTSuccess; pConnectInfo = allocateMqttConnectInfo( NULL ); - __CPROVER_assume( isValidMqttConnectInfo( pConnectInfo ) ); + + /* Avoid a malloc(0U) which causes dereference errors */ + if ( pConnectInfo->clientIdentifierLength == 0U ) { + pConnectInfo->pClientIdentifier = NULL; + } pWillInfo = allocateMqttPublishInfo( NULL ); __CPROVER_assume( isValidMqttPublishInfo( pWillInfo ) ); From e827ee20eed898ecce18611347aa12ebfb8a05e2 Mon Sep 17 00:00:00 2001 From: Kody Stribrny Date: Wed, 29 Oct 2025 15:25:43 -0700 Subject: [PATCH 5/5] Only access when not NULL --- .../MQTT_SerializeConnect/MQTT_SerializeConnect_harness.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/cbmc/proofs/MQTT_SerializeConnect/MQTT_SerializeConnect_harness.c b/test/cbmc/proofs/MQTT_SerializeConnect/MQTT_SerializeConnect_harness.c index e61f7afa1..fea4e8bb1 100644 --- a/test/cbmc/proofs/MQTT_SerializeConnect/MQTT_SerializeConnect_harness.c +++ b/test/cbmc/proofs/MQTT_SerializeConnect/MQTT_SerializeConnect_harness.c @@ -41,7 +41,8 @@ void harness() pConnectInfo = allocateMqttConnectInfo( NULL ); /* Avoid a malloc(0U) which causes dereference errors */ - if ( pConnectInfo->clientIdentifierLength == 0U ) { + if( ( pConnectInfo != NULL ) && ( pConnectInfo->clientIdentifierLength == 0U ) ) + { pConnectInfo->pClientIdentifier = NULL; }