romenskiy2012/curl
1
0
Fork 0
mirror of https://github.com/curl/curl.git synced 2026-08-04 19:26:13 +03:00
Code Issues Projects Releases Packages Wiki Activity

mqtt: validate PINGRESP and DISCONNECT have remaining_length == 0

Browse source 
Per MQTT 3.1.1 sections 3.13.1 and 3.14.1, PINGRESP and DISCONNECT fixed
headers must have remaining_length set to zero. The previous code
dispatched to mqtt->nextstate based on the queued state alone without
validating remaining_length for these no-payload packet types, allowing
a malicious broker to send a PINGRESP with non-zero remaining_length
whose trailing bytes would be interpreted as the payload of whatever
message type was queued (CONNACK, SUBACK, etc.).

The exploitation path turned out to be narrow — curl sends data to the
server the user chose to talk to — but the spec violation and the
resulting protocol-state error are real. Reject the malformed packets
with CURLE_WEIRD_SERVER_REPLY before state dispatch.

Reported-by: Raymond Steen <raymond@vortiqxconsilium.com>
Found by VORTIQ-X VXF Framework
Bug: https://hackerone.com/reports/3702718

Signed-off-by: Raymond Steen <raymond@vortiqxconsilium.com>
Closes #21465
This commit is contained in:
Raymond Steen 2026-04-29 10:27:39 +03:00 • committed by Daniel Stenberg
parent ddb30354f6
commit 2bb5c9b555
No known key found for this signature in database
GPG key ID: 5CC908FDB71E12C2
5 changed files with 183 additions and 9 deletions
Download patch file Download diff file Expand all files Collapse all files

18
lib/mqtt.c
View file

@ -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;

2
tests/data/Makefile.am
View file

@ -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 \

59
tests/data/test2206 Normal file
View file

@ -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>

59
tests/data/test2207 Normal file
View file

@ -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>

54
tests/server/mqttd.c
View file

@ -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;
}