SN_Decode_PublishResp: reject total_len not matching fixed layout

[jmestwa-coder]

What

SN_Decode_PublishResp in src/mqtt_sn_packet.c validates total_len
against rx_buf_len but never against the actual fixed-size layout
defined by the MQTT-SN spec for the four packet types it decodes:

• PUBACK is fixed at 7 octets: Length + MsgType + TopicID(2) +
  MsgID(2) + ReturnCode.
• PUBREC / PUBREL / PUBCOMP are fixed at 4 octets: Length + MsgType +
  MsgID(2).

The relevant code, at src/mqtt_sn_packet.c:1308-1317 on master:

total_len = *rx_payload++;

if(total_len > rx_buf_len) {
    return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER);
}

rec_type = *rx_payload++;

With rx_buf_len = 1 and rx_buf[0] = 0x00 (or 0x01), the function:

1. Reads total_len from byte 0.
2. Passes the total_len > rx_buf_len check (0 > 1 and 1 > 1 are
   both false).
3. Reads rec_type from byte 1 — one byte past the caller-supplied
   bound.

How to reproduce

A page-guard reproducer (mmap of one RW page followed by an adjacent
PROT_NONE page, with rx_buf placed at the last byte of the readable
page) crashes with SIGBUS at the rec_type = *rx_payload++ read on
master:

long page = sysconf(_SC_PAGESIZE);
unsigned char *base = mmap(NULL, page * 2, PROT_READ | PROT_WRITE,
    MAP_PRIVATE | MAP_ANON, -1, 0);
mprotect(base + page, page, PROT_NONE);
unsigned char *rx_buf = base + page - 1;
rx_buf[0] = 0x00;

SN_PublishResp resp;
memset(&resp, 0, sizeof(resp));
(void)SN_Decode_PublishResp(rx_buf, 1, SN_MSG_TYPE_PUBACK, &resp);

Master: process terminates with SIGBUS (exit 138 on macOS).
Patched: returns MQTT_CODE_ERROR_MALFORMED_DATA (-3), no crash.

Fix

Add an in-callee total_len check that mirrors the strict-equality
pattern already used by the surrounding decoders for fixed-size MQTT-SN
packets (e.g. SN_Decode_RegAck uses total_len != 7,
SN_Decode_SubscribeAck uses total_len != 8,
SN_Decode_UnsubscribeAck uses total_len != 4,
SN_Decode_Disconnect and SN_Decode_Ping use total_len != 2):

if (type == SN_MSG_TYPE_PUBACK) {
    if (total_len != 7) {
        return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_MALFORMED_DATA);
    }
}
else if (total_len != 4) {
    return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_MALFORMED_DATA);
}

This rejects the malformed frame before any further field read and is
local to the callee. Well-formed PUBACK (total_len=7) and
PUBREC/PUBREL/PUBCOMP (total_len=4) frames continue to decode
unchanged.

Build / test evidence

• ./autogen.sh && ./configure --enable-sn --disable-tls && make -j4
  succeeds.
• make check unit_tests pass.
  (scripts/firmware.test is pre-existing and requires a live
  test.mosquitto.org connection.)
• A standalone test that decodes a valid 7-byte PUBACK
  ({7,PUBACK,0x01,0x02,0x03,0x04,0x05}) still returns 7 with
  topicId=0x0102, packet_id=0x0304, return_code=0x05.
• A standalone test that decodes a valid 4-byte PUBREL
  ({4,PUBREL,0x10,0x20}) still returns 4 with packet_id=0x1020.
• The page-guard reproducer above changes from SIGBUS to a clean
  MQTT_CODE_ERROR_MALFORMED_DATA return.

[wolfSSL-Bot]

Can one of the admins verify this patch?