Reject null chars in strings

[embhorn]

MQTT 3.1.1 explicitly requires receivers to close the network connection when a Control Packet contains U+0000 in a UTF-8 encoded string.

Credit for reporting this issue:
Xiangdong Li (Student, Beijing University of Posts and Telecommunicationsp[BUPT])

Added tests

decode_publish_rejects_nul_in_topic
decode_connect_rejects_nul_in_client_id
decode_connect_rejects_nul_in_username
decode_connect_rejects_nul_in_password
decode_connect_rejects_nul_in_will_topic
decode_subscribe_rejects_nul_in_filter
decode_publish_v5_rejects_nul_in_string_property
decode_unsubscribe_rejects_nul_in_filter

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #503

Scan targets checked: wolfmqtt-bugs, wolfmqtt-src

No new issues found in the changed files. ✅

[Copilot]

Pull request overview

This PR hardens wolfMQTT’s packet decoding to reject embedded NUL bytes (U+0000 / 0x00) in MQTT UTF-8 strings, aligning with MQTT 3.1.1/5.0 requirements and preventing downstream C-string truncation issues in broker logic.

Changes:

• Add an embedded-NUL rejection check to MqttDecode_String, returning MQTT_CODE_ERROR_MALFORMED_DATA.
• Propagate underlying MqttDecode_String errors through MqttDecode_Publish and MQTT v5 property string decoding (MqttDecode_Props) rather than masking them.
• Add targeted decode tests covering topic names/filters, CONNECT fields, and v5 string properties; add XMEMCHR portability macro.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
wolfmqtt/mqtt_types.h	Adds XMEMCHR wrapper (defaults to memchr) to support the new NUL-scan in decoding.
src/mqtt_packet.c	Rejects embedded NULs in MqttDecode_String and propagates string decode errors from PUBLISH + v5 property parsing.
tests/test_mqtt_packet.c	Adds regression tests ensuring decode paths reject NULs in topic/client-id/credentials/filters and v5 string properties.
src/mqtt_broker.c	Updates internal documentation to reflect the new “no embedded NUL” invariant for protocol-decoded strings.
ChangeLog.md	Documents the behavior/API changes for the pending release.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[aidangarske]

Skoll Multi-Scan Review

Modes: review + review-security
Overall recommendation: REQUEST_CHANGES
Findings: 7 total — 7 posted, 0 skipped
7 finding(s) posted as inline comments (see file-level comments below)

Posted findings

• [High] [review+review-security] CONNECT password path now rejects valid binary payloads containing 0x00 — src/mqtt_packet.c:350-362
• [Medium] [review] v5 string-property NUL test gated behind WOLFMQTT_BROKER — tests/test_mqtt_packet.c:1735-1753, 2374
• [Medium] [review] STRING_PAIR error-propagation branch in MqttDecode_Props has no direct regression test — src/mqtt_packet.c:717-755
• [Medium] [review] Behavior change in MqttDecode_Props error code is a soft API break for property decode callers — src/mqtt_packet.c:645-668, 713-755
• [Low] [review-security] New XMEMCHR usage breaks WOLFMQTT_CUSTOM_STRING builds — wolfmqtt/mqtt_types.h:222-251
• [Low] [review] Test placed under MqttDecode_Subscribe section but tests PUBLISH — tests/test_mqtt_packet.c:1735-1753
• [Low] [review] Comment in mqtt_broker.c overstates the invariant guarantee — src/mqtt_broker.c:177-188

Review generated by Skoll

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #503

Scan targets checked: wolfmqtt-bugs, wolfmqtt-src

No new issues found in the changed files. ✅