From 803db1773d9e98170d718caddbd6544b51a6398a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Afonso=20Janu=C3=A1rio?= Date: Fri, 4 Sep 2026 22:25:28 +0100 Subject: [PATCH] Reject a packet_buffer_size of 0 instead of accepting it The websockets_headers_size/packet_buffer_size config option let through a value of 0, which the http__read() path in http_serv.c then treats as an already-allocated, NUL-terminated header buffer: it calls strlen() on it right away and later writes a terminator at packet_buffer[packet_buffer_size - 1]. Neither of those makes sense for a zero-byte buffer, and both read and write past the tiny allocation mosquitto_calloc(1, 0) actually returns. A remote client can trigger this just by connecting to a builtin WebSocket listener configured this way and sending an ordinary HTTP Upgrade request, no MQTT logic involved. Confirmed under AddressSanitizer: a real network request to a broker started with packet_buffer_size 0 produces a deterministic heap-buffer-overflow in strlen() at http_serv.c:85, aborting the broker. The buffer needs at least one byte to ever hold that terminator, so the config parser now rejects 0 the same way it already rejects anything above UINT16_MAX. Added a regression test alongside the existing packet_buffer_size/websockets_headers_size range checks in 16-config-parse-errors-without-tls.py, and updated the wording of the two existing out-of-range tests to match the new message. --- src/conf.c | 8 ++++++-- test/broker/16-config-parse-errors-without-tls.py | 5 +++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/conf.c b/src/conf.c index 05627df458..f5582b4ac9 100644 --- a/src/conf.c +++ b/src/conf.c @@ -2894,8 +2894,12 @@ static int config__read_file_core(struct mosquitto__config *config, bool reload, if(conf__parse_int(&token, token, &tmp_int, &saveptr)){ return MOSQ_ERR_INVAL; } - if(tmp_int < 0 || tmp_int > UINT16_MAX){ - log__printf(NULL, MOSQ_LOG_WARNING, "Error: Packet buffer size must be between 0 and 65535 inclusive."); + /* A buffer of size 0 can never hold the NUL terminator that + * http__read() and the packet reading code both assume is + * always present, so it isn't a usable value here even + * though it fits in the uint16_t range. */ + if(tmp_int < 1 || tmp_int > UINT16_MAX){ + log__printf(NULL, MOSQ_LOG_WARNING, "Error: Packet buffer size must be between 1 and 65535 inclusive."); return MOSQ_ERR_INVAL; } config->packet_buffer_size = (uint16_t)tmp_int; diff --git a/test/broker/16-config-parse-errors-without-tls.py b/test/broker/16-config-parse-errors-without-tls.py index 469caf08c1..27457257f4 100755 --- a/test/broker/16-config-parse-errors-without-tls.py +++ b/test/broker/16-config-parse-errors-without-tls.py @@ -24,6 +24,7 @@ do_test_broker_failure(conf_file, ["mount_point test/"], port, 3, "Error: The 'mount_point' option requires a listener to be defined first.") # Missing listener config do_test_broker_failure(conf_file, [f"listener {port}","mount_point test/+/"], port, 3, "Error: Invalid 'mount_point' value (test/+/). Does it contain a wildcard character?") # Wildcard in mount point. do_test_broker_failure(conf_file, [f"listener 100000"], port, 3, "Error: Invalid 'port' value (100000).") # Out of range +do_test_broker_failure(conf_file, [f"listener {port}","packet_buffer_size 0"], port, 3, "Error: Packet buffer size must be between 1 and 65535 inclusive.") # A zero-sized buffer can never hold the NUL terminator http__read() assumes is there if mosq_test.check_features(["WITH_UNIX_SOCKETS"]): do_test_broker_failure(conf_file, [f"listener 0"], port, 3, "Error: A listener with port 0 must provide a Unix socket path.") # Missing unix socket do_test_broker_failure(conf_file, [f"listener {port}","protocol"], port, 3, "Error: Empty 'protocol' value in configuration.") # Empty proto @@ -86,8 +87,8 @@ do_test_broker_failure(conf_file, [f"listener {port}", "max_topic_alias_broker -1"], port, 3, "Error: Invalid 'max_topic_alias_broker' value in configuration.") # Invalid value do_test_broker_failure(conf_file, [f"listener {port}", "listener_auto_id_prefix"], port, 3, "Error: Empty 'listener_auto_id_prefix' value in configuration.") # Empty string do_test_broker_failure(conf_file, [f"listener {port}", f"listener_auto_id_prefix {'a'*51}"], port, 3, "Error: 'listener_auto_id_prefix' length must be <= 50.") # Invalid value -do_test_broker_failure(conf_file, ["websockets_headers_size 65536"], port, 3, "Error: Packet buffer size must be between 0 and 65535 inclusive.") # Invalid value -do_test_broker_failure(conf_file, ["websockets_headers_size -1"], port, 3, "Error: Packet buffer size must be between 0 and 65535 inclusive.") # Invalid value +do_test_broker_failure(conf_file, ["websockets_headers_size 65536"], port, 3, "Error: Packet buffer size must be between 1 and 65535 inclusive.") # Invalid value +do_test_broker_failure(conf_file, ["websockets_headers_size -1"], port, 3, "Error: Packet buffer size must be between 1 and 65535 inclusive.") # Invalid value do_test_broker_failure(conf_file, ["memory_limit -1"], port, 3, "Error: Invalid 'memory_limit' value (-1).") # Invalid value do_test_broker_failure(conf_file, ["sys_interval -1"], port, 3, "Error: Invalid 'sys_interval' value (-1).") # Invalid value