Current Behavior
The mqtt-proxy stream plugin can throw an unhandled Lua runtime error while parsing the CONNECT packet, instead of rejecting the connection with a 503 like it does for every other malformed-packet case.
The problem is in decode_variable_byte_int() in apisix/stream/plugins/mqtt-proxy.lua:
local function decode_variable_byte_int(data, offset)
local multiplier = 1
local len = 0
local pos
for i = offset, offset + 3 do
pos = i
local byte = str_byte(data, i, i)
len = len + bit.band(byte, 127) * multiplier
multiplier = multiplier * 128
if bit.band(byte, 128) == 0 then
break
end
end
return len, pos
end
It loops up to 4 bytes reading str_byte(data, i, i) but never checks that i is still inside data. This function is used to decode the MQTT v5 "Properties Length" field (parse_mqtt, line 103), and unlike the client-ID length handling a few lines below it (which correctly checks parsed_pos + client_id_len > #data and bails out), there's no equivalent bounds check here.
If the properties-length bytes have the continuation bit (0x80) set all the way to the end of the buffer that was peeked from the socket, str_byte returns nothing for the out-of-range index, so byte is nil, and bit.band(nil, 127) raises a runtime error. This happens inside the preread phase, so the connection doesn't get the plugin's normal core.log.error(...); return 503 treatment - it just errors out.
Expected Behavior
A malformed/truncated MQTT v5 properties length should be handled the same way the rest of this parser handles malformed input - log an error and return 503 - not raise an uncaught Lua error.
Error Logs
bad argument #1 to 'band' (number expected, got nil)
(raised from bit.band(byte, 127) inside decode_variable_byte_int, apisix/stream/plugins/mqtt-proxy.lua:62)
Steps to Reproduce
- Configure a stream route with the
mqtt-proxy plugin (protocol_name = "MQTT", protocol_level = 5) in front of any upstream.
- Open a raw TCP connection to the route and send this 14-byte MQTT v5 CONNECT packet (hex):
10 0c 00 04 4d 51 54 54 05 02 00 3c 80 80
Breaking it down:
10 - CONNECT packet type
0c - Remaining Length = 12
00 04 4d 51 54 54 - protocol name length (4) + "MQTT"
05 - protocol version 5 (MQTT v5, so the properties field gets parsed)
02 00 3c - connect flags + keepalive (skipped)
80 80 - properties length field, both bytes have the continuation bit set, and the buffer ends right there
decode_variable_byte_int walks past the end of the 14-byte buffer trying to read a third properties-length byte and errors instead of returning 503.
I checked the two mqtt-proxy test files (t/stream-plugin/mqtt-proxy.t, t/stream-plugin/mqtt-proxy2.t) and none of the existing cases cover a properties length whose continuation bytes run past the end of the peeked data, so this isn't caught by CI right now.
Environment
- APISIX version: master (current HEAD around 3.18)
- Operating system: N/A (code-level issue, not environment specific)
- OpenResty / Nginx version: N/A
- etcd version, if relevant: N/A
- APISIX Dashboard version, if relevant: N/A
- Plugin runner version, for issues related to plugin runners: N/A
- LuaRocks version, for installation issues: N/A
Current Behavior
The
mqtt-proxystream plugin can throw an unhandled Lua runtime error while parsing the CONNECT packet, instead of rejecting the connection with a 503 like it does for every other malformed-packet case.The problem is in
decode_variable_byte_int()inapisix/stream/plugins/mqtt-proxy.lua:It loops up to 4 bytes reading
str_byte(data, i, i)but never checks thatiis still insidedata. This function is used to decode the MQTT v5 "Properties Length" field (parse_mqtt, line 103), and unlike the client-ID length handling a few lines below it (which correctly checksparsed_pos + client_id_len > #dataand bails out), there's no equivalent bounds check here.If the properties-length bytes have the continuation bit (0x80) set all the way to the end of the buffer that was peeked from the socket,
str_bytereturns nothing for the out-of-range index, sobyteisnil, andbit.band(nil, 127)raises a runtime error. This happens inside theprereadphase, so the connection doesn't get the plugin's normalcore.log.error(...); return 503treatment - it just errors out.Expected Behavior
A malformed/truncated MQTT v5 properties length should be handled the same way the rest of this parser handles malformed input - log an error and return 503 - not raise an uncaught Lua error.
Error Logs
(raised from
bit.band(byte, 127)insidedecode_variable_byte_int,apisix/stream/plugins/mqtt-proxy.lua:62)Steps to Reproduce
mqtt-proxyplugin (protocol_name = "MQTT",protocol_level = 5) in front of any upstream.Breaking it down:
10- CONNECT packet type0c- Remaining Length = 1200 04 4d 51 54 54- protocol name length (4) + "MQTT"05- protocol version 5 (MQTT v5, so the properties field gets parsed)02 00 3c- connect flags + keepalive (skipped)80 80- properties length field, both bytes have the continuation bit set, and the buffer ends right theredecode_variable_byte_intwalks past the end of the 14-byte buffer trying to read a third properties-length byte and errors instead of returning 503.I checked the two mqtt-proxy test files (
t/stream-plugin/mqtt-proxy.t,t/stream-plugin/mqtt-proxy2.t) and none of the existing cases cover a properties length whose continuation bytes run past the end of the peeked data, so this isn't caught by CI right now.Environment