From e9687e88988a60b8aaf9bc32ee8f2e1e79d7182a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Morgan=20T=C3=B8rvolt?= Date: Sat, 8 Aug 2026 00:47:58 +0200 Subject: [PATCH 1/4] Add regression tests for UTF-8 noncharacter mask and malformed continuation bytes validate_mqtt_utf8_char uses (c & 0xFE) == 0xFE to detect unicode noncharacters. That rejects any where low byte is 0xFE or 0xFF (e.g. U+00FE, U+00FF, U+1F5FE). It should reject the 34 non-characters. pop_front_unichar also never validates continuation bytes. That means malformed sequences like "\xC3z" are considered valid. These test currently fail and show both defects. --- test/unit/string_validation.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/unit/string_validation.cpp b/test/unit/string_validation.cpp index 62a46cd..9dc40a9 100644 --- a/test/unit/string_validation.cpp +++ b/test/unit/string_validation.cpp @@ -57,6 +57,12 @@ BOOST_AUTO_TEST_CASE(utf8_string_validation) { BOOST_CHECK(validate_mqtt_utf8(to_str(0xFDF0)) == validation_result::valid); BOOST_CHECK(validate_mqtt_utf8(to_str(0x1FFFE)) == validation_result::invalid); BOOST_CHECK(validate_mqtt_utf8(to_str(0x1FFFF)) == validation_result::invalid); + + BOOST_CHECK(validate_mqtt_utf8(to_str(0xFE)) == validation_result::valid); + BOOST_CHECK(validate_mqtt_utf8(to_str(0xFF)) == validation_result::valid); + BOOST_CHECK(validate_mqtt_utf8(to_str(0x1F5FE)) == validation_result::valid); + + BOOST_CHECK(validate_mqtt_utf8("\xC3z") == validation_result::invalid); } BOOST_AUTO_TEST_CASE(topic_filter_validation) { From c41ddca09ec3f124f5dbec03c7e5ba54de7ec78c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Morgan=20T=C3=B8rvolt?= Date: Sat, 8 Aug 2026 01:31:40 +0200 Subject: [PATCH 2/4] Fix the bitmask bug, check the extra bytes too validate_mqtt_utf8_char only looked at the low byte for 0xFE or 0xFF instead of checking all the bits it needed to. Correct check is (c & 0xFFFE) == 0xFFFE. pop_front_unichar now checks the extra bytes are actually in range 0x80-0xBF before using them. Bad utf8 like "\xC3z" gets rejected now instead of decoded. Also fixed the 4-byte lead byte mask from 0x1F to 0x07 since only 3 bits are used there. Tests from the last commit pass now. --- include/boost/mqtt5/detail/utf8_mqtt.hpp | 28 +++++++++++++++++------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/include/boost/mqtt5/detail/utf8_mqtt.hpp b/include/boost/mqtt5/detail/utf8_mqtt.hpp index d80ac30..99557c2 100644 --- a/include/boost/mqtt5/detail/utf8_mqtt.hpp +++ b/include/boost/mqtt5/detail/utf8_mqtt.hpp @@ -21,6 +21,10 @@ enum class validation_result : uint8_t { invalid }; +inline bool is_continuation_byte(char c) { + return (c & 0xC0) == 0x80; +} + inline int pop_front_unichar(std::string_view& s) { // assuming that s.length() is > 0 @@ -31,16 +35,26 @@ inline int pop_front_unichar(std::string_view& s) { ch = s[0]; s.remove_prefix(1); } - else if ((n == 0xC0 || n == 0xD0) && s.size() > 1) { + else if ( + (n == 0xC0 || n == 0xD0) && s.size() > 1 && + is_continuation_byte(s[1]) + ) { ch = ((s[0] & 0x1F) << 6) | (s[1] & 0x3F); s.remove_prefix(2); } - else if ((n == 0xE0) && s.size() > 2) { + else if ( + (n == 0xE0) && s.size() > 2 && + is_continuation_byte(s[1]) && is_continuation_byte(s[2]) + ) { ch = ((s[0] & 0x1F) << 12) | ((s[1] & 0x3F) << 6) | (s[2] & 0x3F); s.remove_prefix(3); } - else if ((n == 0xF0) && s.size() > 3) { - ch = ((s[0] & 0x1F) << 18) | ((s[1] & 0x3F) << 12) | + else if ( + (n == 0xF0) && s.size() > 3 && + is_continuation_byte(s[1]) && is_continuation_byte(s[2]) && + is_continuation_byte(s[3]) + ) { + ch = ((s[0] & 0x07) << 18) | ((s[1] & 0x3F) << 12) | ((s[2] & 0x3F) << 6) | (s[3] & 0x3F); s.remove_prefix(4); } @@ -49,8 +63,7 @@ inline int pop_front_unichar(std::string_view& s) { } inline validation_result validate_mqtt_utf8_char(int c) { - constexpr int fe_flag = 0xFE; - constexpr int ff_flag = 0xFF; + constexpr int noncharacter_flag = 0xFFFE; constexpr int multi_lvl_wildcard = '#'; constexpr int single_lvl_wildcard = '+'; @@ -62,8 +75,7 @@ inline validation_result validate_mqtt_utf8_char(int c) { (c < 0x007F || c > 0x009F) && // U+007F...0+009F control characters (c < 0xD800 || c > 0xDFFF) && // U+D800...U+DFFF surrogates (c < 0xFDD0 || c > 0xFDEF) && // U+FDD0...U+FDEF non-characters - (c & fe_flag) != fe_flag && // non-characters - (c & ff_flag) != ff_flag + (c & noncharacter_flag) != noncharacter_flag // non-characters ) return validation_result::valid; From 04937d062bcdfa0ee81a38da439bed6b744ef037 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Morgan=20T=C3=B8rvolt?= Date: Sat, 8 Aug 2026 03:42:18 +0200 Subject: [PATCH 3/4] Add tests for bad continuation bytes and overlong encodings Two of these already pass since the last fix. The three overlong ones don't, since nothing checks for that yet. --- test/unit/string_validation.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/unit/string_validation.cpp b/test/unit/string_validation.cpp index 9dc40a9..67c3fea 100644 --- a/test/unit/string_validation.cpp +++ b/test/unit/string_validation.cpp @@ -63,6 +63,12 @@ BOOST_AUTO_TEST_CASE(utf8_string_validation) { BOOST_CHECK(validate_mqtt_utf8(to_str(0x1F5FE)) == validation_result::valid); BOOST_CHECK(validate_mqtt_utf8("\xC3z") == validation_result::invalid); + BOOST_CHECK(validate_mqtt_utf8("\xE2\x28\xA1") == validation_result::invalid); + BOOST_CHECK(validate_mqtt_utf8("\xF0\x28\x8C\xBC") == validation_result::invalid); + + BOOST_CHECK(validate_mqtt_utf8("\xC1\x81") == validation_result::invalid); + BOOST_CHECK(validate_mqtt_utf8("\xE0\x81\x81") == validation_result::invalid); + BOOST_CHECK(validate_mqtt_utf8("\xF0\x80\x81\x81") == validation_result::invalid); } BOOST_AUTO_TEST_CASE(topic_filter_validation) { From 82d6e4a943d1a112439962353ead943144a9a60e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Morgan=20T=C3=B8rvolt?= Date: Sat, 8 Aug 2026 03:42:30 +0200 Subject: [PATCH 4/4] Reject overlong utf8 encodings pop_front_unichar decoded multi-byte sequences without checking the result was big enough to actually need that many bytes. So something like a 2-byte encoding of a plain ascii letter got accepted. Now it checks the decoded value against the minimum for each length (0x80, 0x800, 0x10000) and rejects it if too small. --- include/boost/mqtt5/detail/utf8_mqtt.hpp | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/include/boost/mqtt5/detail/utf8_mqtt.hpp b/include/boost/mqtt5/detail/utf8_mqtt.hpp index 99557c2..2688a50 100644 --- a/include/boost/mqtt5/detail/utf8_mqtt.hpp +++ b/include/boost/mqtt5/detail/utf8_mqtt.hpp @@ -39,24 +39,33 @@ inline int pop_front_unichar(std::string_view& s) { (n == 0xC0 || n == 0xD0) && s.size() > 1 && is_continuation_byte(s[1]) ) { - ch = ((s[0] & 0x1F) << 6) | (s[1] & 0x3F); - s.remove_prefix(2); + int decoded = ((s[0] & 0x1F) << 6) | (s[1] & 0x3F); + if (decoded >= 0x80) { + ch = decoded; + s.remove_prefix(2); + } } else if ( (n == 0xE0) && s.size() > 2 && is_continuation_byte(s[1]) && is_continuation_byte(s[2]) ) { - ch = ((s[0] & 0x1F) << 12) | ((s[1] & 0x3F) << 6) | (s[2] & 0x3F); - s.remove_prefix(3); + int decoded = ((s[0] & 0x1F) << 12) | ((s[1] & 0x3F) << 6) | (s[2] & 0x3F); + if (decoded >= 0x800) { + ch = decoded; + s.remove_prefix(3); + } } else if ( (n == 0xF0) && s.size() > 3 && is_continuation_byte(s[1]) && is_continuation_byte(s[2]) && is_continuation_byte(s[3]) ) { - ch = ((s[0] & 0x07) << 18) | ((s[1] & 0x3F) << 12) | + int decoded = ((s[0] & 0x07) << 18) | ((s[1] & 0x3F) << 12) | ((s[2] & 0x3F) << 6) | (s[3] & 0x3F); - s.remove_prefix(4); + if (decoded >= 0x10000) { + ch = decoded; + s.remove_prefix(4); + } } return ch;