From 94349aa9326c217e162a61bf16281842a376a919 Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Wed, 4 Feb 2026 11:00:56 +0100 Subject: [PATCH] mqtt: verify Remaining Length for CONNACK and PUBACK Verified in test 1132 Closes #20513 --- lib/mqtt.c | 19 ++++++++++---- tests/data/Makefile.am | 2 +- tests/data/test1132 | 59 ++++++++++++++++++++++++++++++++++++++++++ tests/server/mqttd.c | 16 ++++++++++-- 4 files changed, 88 insertions(+), 8 deletions(-) create mode 100644 tests/data/test1132 diff --git a/lib/mqtt.c b/lib/mqtt.c index 46bfad9596..d12ecae268 100644 --- a/lib/mqtt.c +++ b/lib/mqtt.c @@ -408,10 +408,15 @@ static CURLcode mqtt_verify_connack(struct Curl_easy *data) DEBUGASSERT(mq); if(!mq) return CURLE_FAILED_INIT; + if(mq->remaining_length != 2) { + failf(data, "CONNACK expected Remaining Length 2, got %zu", + mq->remaining_length); + return CURLE_WEIRD_SERVER_REPLY; + } result = mqtt_recv_atleast(data, MQTT_CONNACK_LEN); if(result) - goto fail; + return result; /* verify CONNACK */ DEBUGASSERT(curlx_dyn_len(&mq->recvbuf) >= MQTT_CONNACK_LEN); @@ -422,12 +427,10 @@ static CURLcode mqtt_verify_connack(struct Curl_easy *data) failf(data, "Expected %02x%02x but got %02x%02x", 0x00, 0x00, ptr[0], ptr[1]); curlx_dyn_reset(&mq->recvbuf); - result = CURLE_WEIRD_SERVER_REPLY; - goto fail; + return CURLE_WEIRD_SERVER_REPLY; } mqtt_recv_consume(data, MQTT_CONNACK_LEN); -fail: - return result; + return CURLE_OK; } static CURLcode mqtt_get_topic(struct Curl_easy *data, @@ -511,6 +514,12 @@ static CURLcode mqtt_verify_suback(struct Curl_easy *data) if(!mqtt || !mq) return CURLE_FAILED_INIT; + if(mq->remaining_length != 3) { + failf(data, "SUBACK expected Remaining Length 3, got %zu", + mq->remaining_length); + return CURLE_WEIRD_SERVER_REPLY; + } + result = mqtt_recv_atleast(data, MQTT_SUBACK_LEN); if(result) goto fail; diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am index e089380611..ca44e3d2b0 100644 --- a/tests/data/Makefile.am +++ b/tests/data/Makefile.am @@ -151,7 +151,7 @@ test1093 test1094 test1095 test1096 test1097 test1098 test1099 test1100 \ test1101 test1102 test1103 test1104 test1105 test1106 test1107 test1108 \ test1109 test1110 test1111 test1112 test1113 test1114 test1115 test1116 \ test1117 test1118 test1119 test1120 test1121 test1122 test1123 test1124 \ -test1125 test1126 test1127 test1128 test1129 test1130 test1131 \ +test1125 test1126 test1127 test1128 test1129 test1130 test1131 test1132 \ test1133 test1134 test1135 test1136 test1137 test1138 test1139 test1140 \ test1141 test1142 test1143 test1144 test1145 test1146 test1147 test1148 \ test1149 test1150 test1151 test1152 test1153 test1154 test1155 test1156 \ diff --git a/tests/data/test1132 b/tests/data/test1132 new file mode 100644 index 0000000000..56d7e9214e --- /dev/null +++ b/tests/data/test1132 @@ -0,0 +1,59 @@ + + + + +MQTT +MQTT CONNACK + + + +# Server-side + + +hello + + +00 04 31 31 39 30 68 65 6c 6c 6f 5b 4c 46 5d 0a + + +# Remaining Length must be 2 in a CONNACK + +remlen-CONNACK 3 + + + +# Client-side + + +mqtt + + +mqtt + + +MQTT CONNACK with bad Remaining Length + + +mqtt://%HOSTIP:%MQTTPORT/%TESTNUMBER + + + +# Verify data after the test has been "shot" + +# These are hexadecimal protocol dumps from the client +# Strip out the random part of the client id from the CONNECT message +# before comparison + +s/^(.* 00044d5154540402003c000c6375726c).*/$1/ + + +client CONNECT 18 00044d5154540402003c000c6375726c +server CONNACK 3 20030000 + + +# 8 == CURLE_WEIRD_SERVER_REPLY + +8 + + + diff --git a/tests/server/mqttd.c b/tests/server/mqttd.c index 3e50b351df..be9b9a4256 100644 --- a/tests/server/mqttd.c +++ b/tests/server/mqttd.c @@ -43,13 +43,14 @@ #define MQTT_MSG_DISCONNECT 0xe0 struct mqttd_configurable { + int testnum; unsigned char version; /* initial version byte in the request must match this */ bool publish_before_suback; bool short_publish; bool excessive_remaining; unsigned char error_connack; - int testnum; + unsigned char remlen_connack; }; #define REQUEST_DUMP "server.input" @@ -65,6 +66,7 @@ static void mqttd_resetdefaults(void) m_config.short_publish = FALSE; m_config.excessive_remaining = FALSE; m_config.error_connack = 0; + m_config.remlen_connack = 0; m_config.testnum = 0; } @@ -103,6 +105,13 @@ static void mqttd_getconfig(void) logmsg("error-CONNACK = %d", m_config.error_connack); } } + else if(!strcmp(key, "remlen-CONNACK")) { + pval = value; + if(!curlx_str_number(&pval, &num, 0xff)) { + m_config.remlen_connack = (unsigned char)num; + logmsg("remlen-CONNACK = %d", m_config.remlen_connack); + } + } else if(!strcmp(key, "Testnum")) { pval = value; if(!curlx_str_number(&pval, &num, INT_MAX)) { @@ -158,13 +167,16 @@ static int connack(FILE *dump, curl_socket_t fd) }; ssize_t rc; + if(m_config.remlen_connack) + packet[1] = m_config.remlen_connack; packet[3] = m_config.error_connack; rc = swrite(fd, packet, sizeof(packet)); if(rc > 0) { logmsg("WROTE %zd bytes [CONNACK]", rc); loghex(packet, rc); - logprotocol(FROM_SERVER, "CONNACK", 2, dump, packet, sizeof(packet)); + logprotocol(FROM_SERVER, "CONNACK", packet[1], dump, + packet, sizeof(packet)); } if(rc == sizeof(packet)) { return 0;