diff --git a/ChangeLog.md b/ChangeLog.md index 0c052a9f..a41685b7 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -1,5 +1,39 @@ ## Release Notes +### v2.0.1 (Pending) + +* Security Hardening + - Reject MQTT UTF-8 encoded strings containing U+0000 in `MqttDecode_String` + per [MQTT-1.5.3-2] / [MQTT-1.5.4-2]. Closes an embedded-NUL truncation + attack that allowed broker-side auth bypass, ClientId collision, and + topic-routing confusion when the broker compared decoded strings with + C-string semantics. + - The CONNECT Password field is decoded by a new internal helper + (`MqttDecode_Password`) that applies the same NUL rejection. Per + MQTT-3.1.3.5 the field is Binary Data so the spec does not require + this check, but wolfMQTT compares passwords with `XSTRLEN`/`XSTRCMP`, + so a binary password with an embedded NUL would be silently truncated + and could enable an auth bypass. Spec-compliant clients sending binary + passwords containing 0x00 will be rejected by the broker as a result. + +* API / Behavior Changes + - `MqttDecode_String` may now return `MQTT_CODE_ERROR_MALFORMED_DATA` + when the decoded string contains an embedded NUL byte. Previously + `MQTT_CODE_ERROR_OUT_OF_BUFFER` was the only possible negative return. + - `MqttDecode_Publish` now propagates the underlying error from + `MqttDecode_String` (e.g. `MALFORMED_DATA`) instead of always returning + `MQTT_CODE_ERROR_OUT_OF_BUFFER` on topic decode failure. + - `MqttDecode_Props` propagates `MQTT_CODE_ERROR_MALFORMED_DATA` from + `MqttDecode_String` for v5 STRING and STRING_PAIR property types + (Reason String, Content Type, User Property, etc.). Other negative + returns continue to be mapped to `MQTT_CODE_ERROR_PROPERTY` so client + code that branches on that error code is unaffected. + - New `XMEMCHR(s, c, n)` portability macro added to `mqtt_types.h` + (defaults to `memchr` for standard builds). Builds using + `WOLFMQTT_CUSTOM_STRING` must define `XMEMCHR` themselves, matching + the pattern used for `XSTRLEN`, `XMEMCMP`, etc.; the header emits + an explicit `#error` if it is missing. + ### v2.0.0 (03/20/2026) Release 2.0.0 has been developed according to wolfSSL's development and QA process (see link below) and successfully passed the quality criteria. diff --git a/src/mqtt_broker.c b/src/mqtt_broker.c index 9c2b4fc6..5859c1d4 100644 --- a/src/mqtt_broker.c +++ b/src/mqtt_broker.c @@ -175,8 +175,23 @@ static int BrokerStrCompare(const char* a, const char* b, int cmp_len) #endif /* WOLFMQTT_BROKER_AUTH */ /* Store a string of known length into a BrokerClient field. - * Static mode: copies into fixed-size buffer with truncation. - * Dynamic mode: frees old value, allocates new buffer, copies. */ + * Static mode: copies into fixed-size buffer with truncation. The + * sensitive-wipe variant zeros the full max_len buffer, so it is + * unaffected by string contents. + * Dynamic mode: frees old value, allocates new buffer, copies. The + * sensitive-wipe path uses XSTRLEN(buf)+1, which is correct only when + * stored values contain no embedded NUL. The decode-side guards that + * feed this function enforce that invariant: + * - MqttDecode_String rejects U+0000 per [MQTT-1.5.3-2] / + * [MQTT-1.5.4-2] for UTF-8 fields (client_id, username, topics). + * - MqttDecode_Password applies the same NUL check to the CONNECT + * Password field; that field is Binary Data per MQTT-3.1.3.5 and + * the spec does not require it, but wolfMQTT compares passwords + * with XSTRLEN/XSTRCMP, so the broker-side defensive check here + * is what keeps the "no embedded NUL" assumption intact. + * Do not call the dynamic-mode sensitive path with src bytes that + * bypass those decoders without first capping src_len at the first + * NUL, or the wipe will leave trailing bytes uncleared. */ #ifdef WOLFMQTT_STATIC_MEMORY static void BrokerStore_String(char* dst, int max_len, const char* src, word16 src_len) diff --git a/src/mqtt_packet.c b/src/mqtt_packet.c index 9f5cf73f..7f96d081 100644 --- a/src/mqtt_packet.c +++ b/src/mqtt_packet.c @@ -335,6 +335,8 @@ int MqttEncode_Int(byte* buf, word32 len) /* Returns number of buffer bytes decoded */ /* Returns pointer to string (which is not guaranteed to be null terminated) */ +/* Note: MqttDecode_Password mirrors this implementation for the CONNECT + * Password binary-data field. Keep length and bounds handling in sync. */ int MqttDecode_String(byte *buf, const char **pstr, word16 *pstr_len, word32 buf_len) { int len; @@ -347,6 +349,12 @@ int MqttDecode_String(byte *buf, const char **pstr, word16 *pstr_len, word32 buf return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER); } buf += len; + /* [MQTT-1.5.3-2] / [MQTT-1.5.4-2]: a UTF-8 encoded string MUST NOT + * include U+0000. Reject so downstream C-string handling cannot be + * tricked by an embedded NUL truncating the value. */ + if (str_len > 0 && XMEMCHR(buf, 0x00, str_len) != NULL) { + return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_MALFORMED_DATA); + } if (pstr_len) { *pstr_len = str_len; } @@ -640,7 +648,17 @@ int MqttDecode_Props(MqttPacketType packet, MqttProp** props, byte* pbuf, (const char**)&cur_prop->data_str.str, &cur_prop->data_str.len, (word32)(buf_len - (buf - pbuf))); - if ((tmp >= 0) && ((word32)tmp <= (buf_len - (buf - pbuf)))) { + if (tmp == MQTT_CODE_ERROR_MALFORMED_DATA) { + /* Propagate spec-required NUL rejection so callers can + * distinguish malformed UTF-8 from generic property + * decode failures. Other negative codes keep their + * legacy MQTT_CODE_ERROR_PROPERTY mapping below. */ + rc = tmp; + } + else if (tmp < 0) { + rc = MQTT_TRACE_ERROR(MQTT_CODE_ERROR_PROPERTY); + } + else if ((word32)tmp <= (buf_len - (buf - pbuf))) { buf += tmp; total += tmp; prop_len -= (word32)tmp; @@ -702,7 +720,15 @@ int MqttDecode_Props(MqttPacketType packet, MqttProp** props, byte* pbuf, (const char**)&cur_prop->data_str.str, &cur_prop->data_str.len, (word32)(buf_len - (buf - pbuf))); - if ((tmp >= 0) && ((word32)tmp <= (buf_len - (buf - pbuf)))) { + /* See MQTT_DATA_TYPE_STRING above for the rationale on + * propagating MALFORMED_DATA distinctly from other errors. */ + if (tmp == MQTT_CODE_ERROR_MALFORMED_DATA) { + rc = tmp; + } + else if (tmp < 0) { + rc = MQTT_TRACE_ERROR(MQTT_CODE_ERROR_PROPERTY); + } + else if ((word32)tmp <= (buf_len - (buf - pbuf))) { buf += tmp; total += tmp; prop_len -= (word32)tmp; @@ -711,8 +737,15 @@ int MqttDecode_Props(MqttPacketType packet, MqttProp** props, byte* pbuf, (const char**)&cur_prop->data_str2.str, &cur_prop->data_str2.len, (word32)(buf_len - (buf - pbuf))); - if ((tmp >= 0) && ((word32)tmp <= - (buf_len - (buf - pbuf)))) { + /* See MQTT_DATA_TYPE_STRING above. */ + if (tmp == MQTT_CODE_ERROR_MALFORMED_DATA) { + rc = tmp; + } + else if (tmp < 0) { + rc = MQTT_TRACE_ERROR(MQTT_CODE_ERROR_PROPERTY); + } + else if ((word32)tmp <= + (buf_len - (buf - pbuf))) { buf += tmp; total += tmp; prop_len -= (word32)tmp; @@ -969,6 +1002,39 @@ int MqttEncode_Connect(byte *tx_buf, int tx_buf_len, MqttConnect *mc_connect) } #ifdef WOLFMQTT_BROKER +/* Decode the CONNECT Password field. Password is Binary Data per + * MQTT-3.1.3.5, not a UTF-8 string, so the [MQTT-1.5.3-2] / [MQTT-1.5.4-2] + * U+0000 prohibition does not formally apply. The defensive NUL check is + * applied here because wolfMQTT compares stored passwords with + * XSTRLEN/XSTRCMP, so a binary password with an embedded NUL would be + * silently truncated and could enable an auth bypass. Decoupling this + * from MqttDecode_String keeps the spec-compliant UTF-8 path separate + * from the broker-specific binary-data validation. */ +static int MqttDecode_Password(byte *buf, const char **ppassword, + word16 *ppassword_len, word32 buf_len) +{ + int len; + word16 pass_len; + len = MqttDecode_Num(buf, &pass_len, buf_len); + if (len < 0) { + return len; + } + if ((word32)pass_len > buf_len - (word32)len) { + return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER); + } + buf += len; + if (pass_len > 0 && XMEMCHR(buf, 0x00, pass_len) != NULL) { + return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_MALFORMED_DATA); + } + if (ppassword_len) { + *ppassword_len = pass_len; + } + if (ppassword) { + *ppassword = (char*)buf; + } + return len + pass_len; +} + int MqttDecode_Connect(byte *rx_buf, int rx_buf_len, MqttConnect *mc_connect) { int header_len, remain_len; @@ -1150,7 +1216,7 @@ int MqttDecode_Connect(byte *rx_buf, int rx_buf_len, MqttConnect *mc_connect) } if (packet.flags & MQTT_CONNECT_FLAG_PASSWORD) { - tmp = MqttDecode_String(rx_payload, &mc_connect->password, NULL, + tmp = MqttDecode_Password(rx_payload, &mc_connect->password, NULL, (word32)(rx_buf_len - (rx_payload - rx_buf))); if (tmp < 0) { return tmp; @@ -1464,12 +1530,13 @@ int MqttDecode_Publish(byte *rx_buf, int rx_buf_len, MqttPublish *publish) /* Decode variable header */ variable_len = MqttDecode_String(rx_payload, &publish->topic_name, &publish->topic_name_len, (word32)(rx_buf_len - (rx_payload - rx_buf))); - if ((variable_len >= 0) && (variable_len + header_len <= rx_buf_len)) { - rx_payload += variable_len; + if (variable_len < 0) { + return variable_len; } - else { + if (variable_len + header_len > rx_buf_len) { return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER); } + rx_payload += variable_len; /* If QoS > 0 then get packet Id */ if (publish->qos > MQTT_QOS_0) { diff --git a/tests/test_mqtt_packet.c b/tests/test_mqtt_packet.c index c9f40ff1..4d95976e 100644 --- a/tests/test_mqtt_packet.c +++ b/tests/test_mqtt_packet.c @@ -555,6 +555,23 @@ TEST(decode_publish_malformed_variable_exceeds_remain) ASSERT_EQ(MQTT_CODE_ERROR_OUT_OF_BUFFER, rc); } +/* [MQTT-1.5.3-2] / [MQTT-4.7.3-2]: a topic name containing U+0000 must be + * rejected. Without this check, downstream broker logic uses C-string + * semantics on the stored topic and a publish to "se\0cret" would route + * to subscribers of "se". */ +TEST(decode_publish_rejects_nul_in_topic) +{ + byte buf[] = { 0x30, 10, + 0x00, 0x07, 's', 'e', 0x00, 'c', 'r', 'e', 't', + 'X' }; + MqttPublish pub; + int rc; + + XMEMSET(&pub, 0, sizeof(pub)); + rc = MqttDecode_Publish(buf, (int)sizeof(buf), &pub); + ASSERT_EQ(MQTT_CODE_ERROR_MALFORMED_DATA, rc); +} + #ifdef WOLFMQTT_V5 /* Hand-validated MQTT v5 PUBLISH packet (independent oracle, not produced by * MqttEncode_Publish) so encode and decode cannot hide a shared bug: @@ -599,6 +616,77 @@ TEST(decode_publish_v5_content_type_property) MqttProps_Free(pub.props); } + +/* [MQTT-1.5.4-2]: an embedded NUL in a v5 STRING property must be + * rejected. Uses a PUBLISH packet with a Content Type property whose + * value contains 0x00. MqttDecode_Props propagates MALFORMED_DATA from + * MqttDecode_String distinctly from generic MQTT_CODE_ERROR_PROPERTY. */ +TEST(decode_publish_v5_rejects_nul_in_string_property) +{ + /* Wire: PUBLISH QoS 0, remain_len=13, topic "a/b", props_len=7, + * prop 0x03 CONTENT_TYPE, str_len=4, "t\0xt". No payload. */ + byte buf[] = { + 0x30, 13, + 0x00, 0x03, 'a', '/', 'b', + 0x07, + 0x03, 0x00, 0x04, 't', 0x00, 'x', 't' + }; + MqttPublish pub; + int rc; + + XMEMSET(&pub, 0, sizeof(pub)); + pub.protocol_level = MQTT_CONNECT_PROTOCOL_LEVEL_5; + rc = MqttDecode_Publish(buf, (int)sizeof(buf), &pub); + ASSERT_EQ(MQTT_CODE_ERROR_MALFORMED_DATA, rc); + MqttProps_Free(pub.props); +} + +/* STRING_PAIR (USER_PROPERTY) NUL rejection — first string of pair. The + * MqttDecode_Props path runs MqttDecode_String twice for STRING_PAIR; + * this pins coverage on the first sub-decode propagating MALFORMED_DATA. */ +TEST(decode_publish_v5_rejects_nul_in_user_prop_key) +{ + /* Wire: PUBLISH QoS 0, remain_len=14, topic "a/b", props_len=8, + * prop 0x26 USER_PROPERTY, key_len=2 "k\0", val_len=1 "v". */ + byte buf[] = { + 0x30, 14, + 0x00, 0x03, 'a', '/', 'b', + 0x08, + 0x26, 0x00, 0x02, 'k', 0x00, + 0x00, 0x01, 'v' + }; + MqttPublish pub; + int rc; + + XMEMSET(&pub, 0, sizeof(pub)); + pub.protocol_level = MQTT_CONNECT_PROTOCOL_LEVEL_5; + rc = MqttDecode_Publish(buf, (int)sizeof(buf), &pub); + ASSERT_EQ(MQTT_CODE_ERROR_MALFORMED_DATA, rc); + MqttProps_Free(pub.props); +} + +/* STRING_PAIR (USER_PROPERTY) NUL rejection — second string of pair. + * Pins coverage on the second sub-decode propagating MALFORMED_DATA. */ +TEST(decode_publish_v5_rejects_nul_in_user_prop_value) +{ + /* Wire: PUBLISH QoS 0, remain_len=14, topic "a/b", props_len=8, + * prop 0x26 USER_PROPERTY, key_len=1 "k", val_len=2 "v\0". */ + byte buf[] = { + 0x30, 14, + 0x00, 0x03, 'a', '/', 'b', + 0x08, + 0x26, 0x00, 0x01, 'k', + 0x00, 0x02, 'v', 0x00 + }; + MqttPublish pub; + int rc; + + XMEMSET(&pub, 0, sizeof(pub)); + pub.protocol_level = MQTT_CONNECT_PROTOCOL_LEVEL_5; + rc = MqttDecode_Publish(buf, (int)sizeof(buf), &pub); + ASSERT_EQ(MQTT_CODE_ERROR_MALFORMED_DATA, rc); + MqttProps_Free(pub.props); +} #endif /* WOLFMQTT_V5 */ /* ============================================================================ @@ -1501,6 +1589,126 @@ TEST(decode_connect_v311_with_lwt) ASSERT_NOT_NULL(dec_lwt.buffer); ASSERT_EQ(0, XMEMCMP(dec_lwt.buffer, lwt_payload, sizeof(lwt_payload))); } + +/* [MQTT-1.5.3-2]: an embedded NUL in the ClientId must be rejected. + * Otherwise BrokerClient_FindByClientId() (which uses XSTRCMP) will treat + * "ad\0min" as "ad" and collide with an existing "ad" session. */ +TEST(decode_connect_rejects_nul_in_client_id) +{ + byte buf[] = { + 0x10, 0x12, /* CONNECT, remain_len = 18 */ + 0x00, 0x04, 'M', 'Q', 'T', 'T', /* protocol name */ + 0x04, /* protocol level v3.1.1 */ + 0x02, /* flags: clean_session */ + 0x00, 0x3C, /* keep alive */ + 0x00, 0x06, 'a', 'd', 0x00, 'm', 'i', 'n' /* client_id with NUL */ + }; + MqttConnect dec; + int rc; + + XMEMSET(&dec, 0, sizeof(dec)); + rc = MqttDecode_Connect(buf, (int)sizeof(buf), &dec); + ASSERT_EQ(MQTT_CODE_ERROR_MALFORMED_DATA, rc); +} + +/* [MQTT-1.5.3-2]: an embedded NUL in the username must be rejected. + * Otherwise BrokerStrCompare() (which uses XSTRLEN) will treat + * "us\0er" as "us" and accept it against a configured "us" credential. */ +TEST(decode_connect_rejects_nul_in_username) +{ + byte buf[] = { + 0x10, 0x15, /* CONNECT, remain_len = 21 */ + 0x00, 0x04, 'M', 'Q', 'T', 'T', + 0x04, + 0x82, /* clean_session + USERNAME */ + 0x00, 0x3C, + 0x00, 0x02, 'c', '1', /* client_id "c1" */ + 0x00, 0x05, 'u', 's', 0x00, 'e', 'r' /* username with NUL */ + }; + MqttConnect dec; + int rc; + + XMEMSET(&dec, 0, sizeof(dec)); + rc = MqttDecode_Connect(buf, (int)sizeof(buf), &dec); + ASSERT_EQ(MQTT_CODE_ERROR_MALFORMED_DATA, rc); +} + +/* [MQTT-1.5.3-2]: an embedded NUL in the password must be rejected. + * Same auth-bypass mechanism as the username test, applied to the + * password field. */ +TEST(decode_connect_rejects_nul_in_password) +{ + byte buf[] = { + 0x10, 0x16, /* CONNECT, remain_len = 22 */ + 0x00, 0x04, 'M', 'Q', 'T', 'T', + 0x04, + 0xC2, /* clean_session + USER + PASS */ + 0x00, 0x3C, + 0x00, 0x02, 'c', '1', + 0x00, 0x01, 'u', /* username "u" */ + 0x00, 0x03, 'p', 0x00, 'w' /* password with NUL */ + }; + MqttConnect dec; + int rc; + + XMEMSET(&dec, 0, sizeof(dec)); + rc = MqttDecode_Connect(buf, (int)sizeof(buf), &dec); + ASSERT_EQ(MQTT_CODE_ERROR_MALFORMED_DATA, rc); +} + +/* [MQTT-1.5.3-2] / [MQTT-4.7.3-2]: a Will Topic with embedded NUL must + * be rejected. The same C-string truncation that affects PUBLISH topics + * applies to Will Topics persisted by the broker. */ +TEST(decode_connect_rejects_nul_in_will_topic) +{ + byte buf[] = { + 0x10, 0x16, /* CONNECT, remain_len = 22 */ + 0x00, 0x04, 'M', 'Q', 'T', 'T', + 0x04, + 0x06, /* clean_session + WILL_FLAG */ + 0x00, 0x3C, + 0x00, 0x02, 'c', '1', /* client_id */ + 0x00, 0x03, 't', 0x00, 'p', /* will topic with NUL */ + 0x00, 0x01, 'X' /* will payload */ + }; + MqttConnect dec; + MqttMessage lwt; + int rc; + + XMEMSET(&dec, 0, sizeof(dec)); + XMEMSET(&lwt, 0, sizeof(lwt)); + dec.lwt_msg = &lwt; + rc = MqttDecode_Connect(buf, (int)sizeof(buf), &dec); + ASSERT_EQ(MQTT_CODE_ERROR_MALFORMED_DATA, rc); +} + +#ifdef WOLFMQTT_V5 +/* MQTT v5 has a different decode path than v3.1.1 (properties walked + * before client_id, separate LWT properties), but the NUL rejection + * lives inside MqttDecode_String / MqttDecode_Password and so applies + * uniformly. This pins coverage on the v5 branch so a future refactor + * cannot quietly bypass the check on one protocol level. */ +TEST(decode_connect_v5_rejects_nul_in_client_id) +{ + byte buf[] = { + 0x10, 0x13, /* CONNECT, remain_len = 19 */ + 0x00, 0x04, 'M', 'Q', 'T', 'T', + 0x05, /* protocol level v5 */ + 0x02, /* flags: clean_session */ + 0x00, 0x3C, /* keep alive */ + 0x00, /* props_len = 0 (VBI) */ + 0x00, 0x06, 'a', 'd', 0x00, 'm', 'i', 'n' /* client_id with NUL */ + }; + MqttConnect dec; + int rc; + + XMEMSET(&dec, 0, sizeof(dec)); + dec.protocol_level = MQTT_CONNECT_PROTOCOL_LEVEL_5; + rc = MqttDecode_Connect(buf, (int)sizeof(buf), &dec); + ASSERT_EQ(MQTT_CODE_ERROR_MALFORMED_DATA, rc); + MqttProps_Free(dec.props); +} +#endif /* WOLFMQTT_V5 */ #endif /* WOLFMQTT_BROKER */ /* ============================================================================ @@ -1564,6 +1772,28 @@ TEST(decode_subscribe_v311_qos3_reserved) ASSERT_EQ(MQTT_QOS_3, topic_arr[0].qos); } +/* [MQTT-1.5.3-2] / [MQTT-4.7.3-2]: a topic filter containing U+0000 must + * be rejected. Without this check, a stored filter "a\0b" would match + * topic "a" once iteration hits the embedded NUL in BrokerTopicMatch. */ +TEST(decode_subscribe_rejects_nul_in_filter) +{ + byte rx_buf[] = { + 0x82, 0x08, + 0x00, 0x01, /* packet_id */ + 0x00, 0x03, 'a', 0x00, 'b', /* filter "a\0b" */ + 0x00 /* options: QoS 0 */ + }; + MqttSubscribe sub; + MqttTopic topic_arr[1]; + int rc; + + XMEMSET(&sub, 0, sizeof(sub)); + XMEMSET(topic_arr, 0, sizeof(topic_arr)); + sub.topics = topic_arr; + rc = MqttDecode_Subscribe(rx_buf, (int)sizeof(rx_buf), &sub); + ASSERT_EQ(MQTT_CODE_ERROR_MALFORMED_DATA, rc); +} + #ifdef WOLFMQTT_V5 /* [MQTT-3.8.3] v5 SUBSCRIBE options byte carries QoS (bits 0-1), No Local * (bit 2), Retain As Published (bit 3), and Retain Handling (bits 4-5). @@ -1596,6 +1826,31 @@ TEST(decode_subscribe_v5_options_byte_qos_extracted) ASSERT_EQ(MQTT_QOS_1, topic_arr[0].qos); } #endif /* WOLFMQTT_V5 */ + +/* ============================================================================ + * MqttDecode_Unsubscribe (broker-side) + * ============================================================================ */ + +/* [MQTT-1.5.3-2] / [MQTT-4.7.3-2]: a topic filter containing U+0000 in an + * UNSUBSCRIBE must be rejected — MqttDecode_Unsubscribe shares the same + * MqttDecode_String chokepoint that SUBSCRIBE uses. */ +TEST(decode_unsubscribe_rejects_nul_in_filter) +{ + byte rx_buf[] = { + 0xA2, 0x07, + 0x00, 0x01, /* packet_id */ + 0x00, 0x03, 'a', 0x00, 'b' /* filter "a\0b" */ + }; + MqttUnsubscribe unsub; + MqttTopic topic_arr[1]; + int rc; + + XMEMSET(&unsub, 0, sizeof(unsub)); + XMEMSET(topic_arr, 0, sizeof(topic_arr)); + unsub.topics = topic_arr; + rc = MqttDecode_Unsubscribe(rx_buf, (int)sizeof(rx_buf), &unsub); + ASSERT_EQ(MQTT_CODE_ERROR_MALFORMED_DATA, rc); +} #endif /* WOLFMQTT_BROKER */ /* ============================================================================ @@ -2127,8 +2382,12 @@ void run_mqtt_packet_tests(void) RUN_TEST(decode_publish_qos1_valid); RUN_TEST(decode_publish_qos0_zero_payload); RUN_TEST(decode_publish_malformed_variable_exceeds_remain); + RUN_TEST(decode_publish_rejects_nul_in_topic); #ifdef WOLFMQTT_V5 RUN_TEST(decode_publish_v5_content_type_property); + RUN_TEST(decode_publish_v5_rejects_nul_in_string_property); + RUN_TEST(decode_publish_v5_rejects_nul_in_user_prop_key); + RUN_TEST(decode_publish_v5_rejects_nul_in_user_prop_value); #endif /* MqttDecode_ConnectAck */ @@ -2178,13 +2437,24 @@ void run_mqtt_packet_tests(void) RUN_TEST(decode_connect_wrong_protocol_name); RUN_TEST(decode_connect_wrong_protocol_length); RUN_TEST(decode_connect_v311_with_lwt); + RUN_TEST(decode_connect_rejects_nul_in_client_id); + RUN_TEST(decode_connect_rejects_nul_in_username); + RUN_TEST(decode_connect_rejects_nul_in_password); + RUN_TEST(decode_connect_rejects_nul_in_will_topic); +#ifdef WOLFMQTT_V5 + RUN_TEST(decode_connect_v5_rejects_nul_in_client_id); +#endif /* MqttDecode_Subscribe */ RUN_TEST(decode_subscribe_v311_single_topic); RUN_TEST(decode_subscribe_v311_qos3_reserved); + RUN_TEST(decode_subscribe_rejects_nul_in_filter); #ifdef WOLFMQTT_V5 RUN_TEST(decode_subscribe_v5_options_byte_qos_extracted); #endif + + /* MqttDecode_Unsubscribe */ + RUN_TEST(decode_unsubscribe_rejects_nul_in_filter); #endif /* QoS 2 ack arithmetic */ diff --git a/wolfmqtt/mqtt_types.h b/wolfmqtt/mqtt_types.h index 2817e4a2..1ce8a94b 100644 --- a/wolfmqtt/mqtt_types.h +++ b/wolfmqtt/mqtt_types.h @@ -264,6 +264,19 @@ enum MqttPacketResponseCodes { #endif #endif +/* XMEMCHR backstop. Standard builds and existing custom-string ports + * (which already pull in via other paths) keep building + * without changes. Custom-string ports that intentionally avoid + * get an explicit #error directing them to define XMEMCHR + * themselves, instead of a confusing missing-header diagnostic. */ +#ifndef XMEMCHR + #ifdef WOLFMQTT_CUSTOM_STRING + #error "WOLFMQTT_CUSTOM_STRING set: please define XMEMCHR" + #else + #define XMEMCHR(s,c,n) memchr((s),(c),(n)) + #endif +#endif + #ifndef WOLFMQTT_CUSTOM_MALLOC #ifndef WOLFMQTT_MALLOC #define WOLFMQTT_MALLOC(s) malloc((s))