bug: mqtt-proxy stream plugin crashes on malformed MQTT v5 properties length

Open Beginner friendly
#13,952 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
86/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
lua

Research direction

Start in apisix/stream/plugins/mqtt-proxy.lua at decode_variable_byte_int() and its use in parse_mqtt. Review the malformed-input cases in t/stream-plugin/mqtt-proxy.t and t/stream-plugin/mqtt-proxy2.t, then exercise the supplied truncated MQTT v5 packet. Done means the case is covered by tests and returns 503 with error logging instead of an uncaught Lua runtime error.

Written by the indexing model from the issue text.

Description

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
  1. Configure a stream route with the mqtt-proxy plugin (protocol_name = "MQTT", protocol_level = 5) in front of any upstream.
  2. 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
  1. 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
Dominant language
Lua
Stars
17.1k
Forks
2.9k
Avg merge
2d 22h
Merged PRs (30d)
51

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from apache/apisix

All issues in apache/apisix

Similar issues

More Lua issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.