Skip to content

mqtt: reject control bytes in the topic#22112

Closed
alhudz wants to merge 1 commit into
curl:masterfrom
alhudz:mqtt-topic-reject-ctrl
Closed

mqtt: reject control bytes in the topic#22112
alhudz wants to merge 1 commit into
curl:masterfrom
alhudz:mqtt-topic-reject-ctrl

Conversation

@alhudz

@alhudz alhudz commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Repro: mqtt://host/aa%00bb (any percent-encoded control byte in the topic, including a nul).

Cause: mqtt_get_topic URL-decodes the topic with REJECT_NADA, so the byte is copied verbatim into the length-prefixed SUBSCRIBE/PUBLISH packet and goes out on the wire (observed 820a0001 0005 6161006262 00, topic aa\0bb). A nul in a topic name is forbidden by MQTT 3.1.1 [MQTT-1.5.4-2], so a conformant broker treats it as a protocol error.

Fix: decode with REJECT_CTRL, matching every other URL scheme that turns a path component into a command argument (ftp, imap, pop3, smtp, smb, dict all use it). test2109 covers it.

@github-actions github-actions Bot added the tests label Jun 19, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens curl’s MQTT URL parsing by rejecting percent-decoded control bytes in the topic path component, preventing invalid MQTT topic names (notably embedded NUL) from being serialized into SUBSCRIBE/PUBLISH packets and causing broker protocol errors.

Changes:

  • Decode MQTT topics using REJECT_CTRL instead of REJECT_NADA to reject decoded control bytes (< 0x20).
  • Add a new regression test (test2109) that reproduces mqtt://host/aa%00bb and asserts failure before SUBSCRIBE is sent.
  • Register the new test in the test suite makefile.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
lib/mqtt.c Switch topic URL-decoding to REJECT_CTRL so decoded control bytes are rejected.
tests/data/test2109 New MQTT test that ensures an embedded decoded NUL in the topic is rejected (CURLE_URL_MALFORMAT) before SUBSCRIBE.
tests/data/Makefile.am Adds test2109 to the TESTCASES list so it runs in CI.

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

@bagder

bagder commented Jun 25, 2026

Copy link
Copy Markdown
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

3 participants