← Commit list

2bb5c9b555

Raymond Steen
Raymond Steen
2026-04-30
SUCCESS Duration: 232s

Subject

mqtt: validate PINGRESP and DISCONNECT have remaining_length == 0

Binary

No binary (build failed)

Architecture & Compiler Flags

Architecture: x86_64-linux-gnu Compiler: gcc (Ubuntu 11.4.0-1ubuntu1~22.04.3) 11.4.0

—

Build Log

(log yok)

Commit Diff GitHub →

lib/mqtt.c +18 -0tests/data/Makefile.am +1 -1tests/data/test2206 +59 -0tests/data/test2207 +59 -0tests/server/mqttd.c +46 -8
lib/mqtt.c+18 -0
@@ -892,6 +892,24 @@ static CURLcode mqtt_doing(struct Curl_easy *data, bool *done)
break;
}
mq->npacket = 0;
+ /* PINGRESP and DISCONNECT must have remaining_length == 0 and
+ * reserved bits (low nibble) must be zero per MQTT 3.1.1
+ * sections 2.2.2, 3.13.1 and 3.14.1. Reject before state
+ * dispatch to prevent nextstate confusion. */
+ {
+ const unsigned char type = mq->firstbyte & 0xF0;
+ const unsigned char reserved = mq->firstbyte & 0x0F;
+ if((type == MQTT_MSG_DISCONNECT || type == MQTT_MSG_PINGRESP) &&
+ (mq->remaining_length || reserved)) {
+ failf(data,
+ "Broker sent malformed %s "
+ "(remaining_length=%zu, header byte=0x%02x)",
+ type == MQTT_MSG_DISCONNECT ? "DISCONNECT" : "PINGRESP",
+ mq->remaining_length, mq->firstbyte);
+ result = CURLE_WEIRD_SERVER_REPLY;
+ break;
+ }
+ }
if(mq->remaining_length) {
mqstate(data, mqtt->nextstate, MQTT_NOSTATE);
break;
tests/data/Makefile.am+1 -1
@@ -261,7 +261,7 @@ test2080 test2081 test2082 test2083 test2084 test2085 test2086 test2087 \
test2088 test2089 test2090 test2091 \
test2100 test2101 test2102 test2103 test2104 \
\
-test2200 test2201 test2202 test2203 test2204 test2205 \
+test2200 test2201 test2202 test2203 test2204 test2205 test2206 test2207 \
\
test2300 test2301 test2302 test2303 test2304 test2306 test2307 test2308 \
test2309 \
tests/data/test2206+59 -0
@@ -0,0 +1,59 @@
+<?xml version="1.0" encoding="US-ASCII"?>
+<testcase>
+<info>
+<keywords>
+MQTT
+MQTT SUBSCRIBE
+</keywords>
+</info>
+
+# Server-side
+<reply>
+<data nocheck="yes">
+hello
+</data>
+
+# Send a PINGRESP (0xD0) with remaining_length=2 in place of the
+# expected CONNACK. MQTT 3.1.1 s. 3.13.1 requires PINGRESP to have
+# remaining_length == 0. Curl must reject this with
+# CURLE_WEIRD_SERVER_REPLY rather than dispatching to the CONNACK
+# handler.
+<servercmd>
+PINGRESP-as-CONNACK TRUE
+</servercmd>
+</reply>
+
+# Client-side
+<client>
+<features>
+mqtt
+</features>
+<server>
+mqtt
+</server>
+<name>
+MQTT reject PINGRESP with nonzero remaining_length in place of CONNACK
+</name>
+<command option="binary-trace">
+mqtt://%HOSTIP:%MQTTPORT/%TESTNUMBER
+</command>
+</client>
+
+# Verify data after the test has been "shot"
+<verify>
+# Strip out the random part of the client id from the CONNECT message
+# before comparison
+<strippart>
+s/^(.* 00044d5154540402003c000c6375726c).*/$1/
+</strippart>
+<protocol>
+client CONNECT 18 00044d5154540402003c000c6375726c
+server PINGRESP-as-CONNACK 2 d0020000
+</protocol>
+
+# 8 is CURLE_WEIRD_SERVER_REPLY
+<errorcode>
+8
+</errorcode>
+</verify>
+</testcase>
tests/data/test2207+59 -0
@@ -0,0 +1,59 @@
+<?xml version="1.0" encoding="US-ASCII"?>
+<testcase>
+<info>
+<keywords>
+MQTT
+MQTT SUBSCRIBE
+</keywords>
+</info>
+
+# Server-side
+<reply>
+<data nocheck="yes">
+hello
+</data>
+
+# Send a DISCONNECT with remaining_length=2 after the PUBLISH.
+# MQTT 3.1.1 s. 3.14.1 requires DISCONNECT to have remaining_length == 0.
+# Curl must reject this with CURLE_WEIRD_SERVER_REPLY.
+<servercmd>
+DISCONNECT-malformed TRUE
+</servercmd>
+</reply>
+
+# Client-side
+<client>
+<features>
+mqtt
+</features>
+<server>
+mqtt
+</server>
+<name>
+MQTT reject DISCONNECT with nonzero remaining_length
+</name>
+<command option="binary-trace">
+mqtt://%HOSTIP:%MQTTPORT/%TESTNUMBER
+</command>
+</client>
+
+# Verify data after the test has been "shot"
+<verify>
+<strippart>
+s/^(.* 00044d5154540402003c000c6375726c).*/$1/
+</strippart>
+<protocol>
+client CONNECT 18 00044d5154540402003c000c6375726c
+server CONNACK 2 20020000
+client SUBSCRIBE 9 000100043232303700
+server SUBACK 3 9003000100
+server PUBLISH c 300c00043232303768656c6c6f0a
+server DISCONNECT-malformed 2 e0020000
+</protocol>
+
+# 8 is CURLE_WEIRD_SERVER_REPLY
+<errorcode>
+8
+</errorcode>
+</verify>
+</testcase>
tests/server/mqttd.c+46 -8
@@ -40,6 +40,7 @@
/* #define MQTT_MSG_PUBACK 0x40 */
#define MQTT_MSG_SUBSCRIBE 0x82
#define MQTT_MSG_SUBACK 0x90
+#define MQTT_MSG_PINGRESP 0xd0
#define MQTT_MSG_DISCONNECT 0xe0
struct mqttd_configurable {
@@ -49,6 +50,8 @@ struct mqttd_configurable {
bool publish_before_suback;
bool short_publish;
bool excessive_remaining;
+ bool pingresp_as_connack; /* send PINGRESP with payload instead of CONNACK */
+ bool disconnect_malformed; /* DISCONNECT with nonzero remlen */
unsigned char error_connack;
unsigned char remlen_connack;
};
@@ -65,6 +68,8 @@ static void mqttd_resetdefaults(void)
m_config.publish_before_suback = FALSE;
m_config.short_publish = FALSE;
m_config.excessive_remaining = FALSE;
+ m_config.pingresp_as_connack = FALSE;
+ m_config.disconnect_malformed = FALSE;
m_config.error_connack = 0;
m_config.remlen_connack = 0;
m_config.testnum = 0;
@@ -98,6 +103,14 @@ static void mqttd_getconfig(void)
logmsg("short-PUBLISH set");
m_config.short_publish = TRUE;
}
+ else if(!strcmp(key, "PINGRESP-as-CONNACK")) {
+ logmsg("PINGRESP-as-CONNACK set");
+ m_config.pingresp_as_connack = TRUE;
+ }
+ else if(!strcmp(key, "DISCONNECT-malformed")) {
+ logmsg("DISCONNECT-malformed set");
+ m_config.disconnect_malformed = TRUE;
+ }
else if(!strcmp(key, "error-CONNACK")) {
pval = value;
if(!curlx_str_number(&pval, &num, 0xff)) {
@@ -166,6 +179,16 @@ static int connack(FILE *dump, curl_socket_t fd)
0x00, 0x00
};
ssize_t rc;
+ const char *label = "CONNACK";
+
+ if(m_config.pingresp_as_connack) {
+ /* Send a PINGRESP (0xD0) with remaining_length=2 and payload
+ mimicking a successful CONNACK. MQTT 3.1.1 s. 3.13.1 requires
+ PINGRESP to have remaining_length=0, so this is malformed. */
+ packet[0] = MQTT_MSG_PINGRESP;
+ label = "PINGRESP-as-CONNACK";
+ logmsg("Sending malformed PINGRESP in place of CONNACK");
+ }
if(m_config.remlen_connack)
packet[1] = m_config.remlen_connack;
@@ -173,10 +196,10 @@ static int connack(FILE *dump, curl_socket_t fd)
rc = swrite(fd, packet, sizeof(packet));
if(rc > 0) {
- logmsg("WROTE %zd bytes [CONNACK]", rc);
+ logmsg("WROTE %zd bytes [%s]", rc, label);
loghex(packet, rc);
- logprotocol(FROM_SERVER, "CONNACK", packet[1], dump,
- packet, sizeof(packet));
+ logprotocol(FROM_SERVER, label, packet[1], dump,
+ packet, rc);
}
if(rc == sizeof(packet)) {
return 0;
@@ -235,15 +258,30 @@ static int disconnect(FILE *dump, curl_socket_t fd)
{
unsigned char packet[] = {
MQTT_MSG_DISCONNECT, 0x00,
+ 0x00, 0x00 /* extra bytes for malformed variant */
};
- ssize_t rc = swrite(fd, packet, sizeof(packet));
- if(rc == sizeof(packet)) {
- logmsg("WROTE %zd bytes [DISCONNECT]", rc);
+ size_t pktlen = 2;
+ const char *label = "DISCONNECT";
+ ssize_t rc;
+
+ if(m_config.disconnect_malformed) {
+ /* Send DISCONNECT with remaining_length=2 (must be 0 per spec) */
+ packet[1] = 0x02;
+ pktlen = 4;
+ label = "DISCONNECT-malformed";
+ logmsg("Sending malformed DISCONNECT with nonzero remaining_length");
+ }
+
+ rc = swrite(fd, packet, pktlen);
+ if(rc > 0) {
+ logmsg("WROTE %zd bytes [%s]", rc, label);
loghex(packet, rc);
- logprotocol(FROM_SERVER, "DISCONNECT", 0, dump, packet, rc);
+ logprotocol(FROM_SERVER, label, packet[1], dump, packet, rc);
+ }
+ if(rc == (ssize_t)pktlen) {
return 0;
}
- logmsg("Failed sending [DISCONNECT]");
+ logmsg("Failed sending [%s]", label);
return 1;
}