SN_Decode_Header: reject extended-length frame with total_len <= header size

[dxbjavid]

What

src/mqtt_sn_packet.c::SN_Decode_Header reads one byte past its caller-supplied
rx_buf / rx_buf_len bound when the peer uses the MQTT-SN
extended-length header form (0x01 [len_hi] [len_lo]) and the decoded
total_len is exactly equal to rx_buf_len.

Why it's a bug

src/mqtt_sn_packet.c:98-125 (pre-patch):

if (rx_buf == NULL || rx_buf_len < MQTT_PACKET_HEADER_MIN_SIZE) { ... }

total_len = *rx_buf++;                              /* read offset 0 */
if (total_len == SN_PACKET_LEN_IND) {
    rc = MqttDecode_Num(rx_buf, &total_len, rx_buf_len - 1);
    if (rc < 0) return rc;
    rx_buf += rc;                                   /* now at offset 3 */
}

if (total_len > rx_buf_len) {                       /* only bound check */
    return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER);
}

packet_type = (SN_MsgType)*rx_buf++;                /* OOB read */

With a 3-byte input 01 00 03, total_len decodes to 3 and the
total_len > rx_buf_len test (3 > 3) is false, so the function proceeds
to read the message-type byte at rx_buf[3] — one byte past the
caller's stated bound.

SN_Decode_Publish already carries the matching guard
(total_len < (word16)(rx_payload - rx_buf) at src/mqtt_sn_packet.c:1248),
so this patch follows the same pattern.

Reproducer

Drop the program below alongside the wolfMQTT source tree and build it
straight against the sources so the bound check is exact. To make the
out-of-bounds read crash deterministically (without depending on a
sanitizer), the buffer is placed at the end of a writable page and the
following page is set PROT_NONE:

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <sys/mman.h>
#include <sys/wait.h>

#include "wolfmqtt/mqtt_client.h"
#include "wolfmqtt/mqtt_sn_packet.h"

/* tiny link stubs */
word32 MqttClient_Flags(struct _MqttClient* c, word32 a, word32 b) {
    (void)c; (void)a; (void)b; return 0;
}
int MqttSocket_Read(struct _MqttClient* c, byte* b, int l, int t) {
    (void)c; (void)b; (void)l; (void)t; return -1;
}
int MqttSocket_Peek(struct _MqttClient* c, byte* b, int l, int t) {
    (void)c; (void)b; (void)l; (void)t; return -1;
}
int MqttSocket_Write(struct _MqttClient* c, const byte* b, int l, int t) {
    (void)c; (void)b; (void)l; (void)t; return -1;
}

int main(void) {
    long pg = sysconf(_SC_PAGESIZE);
    unsigned char *region = mmap(NULL, pg*2, PROT_READ|PROT_WRITE,
                                 MAP_PRIVATE|MAP_ANON, -1, 0);
    mprotect(region + pg, pg, PROT_NONE);
    unsigned char *buf = region + pg - 3;
    buf[0] = 0x01; buf[1] = 0x00; buf[2] = 0x03;

    SN_MsgType t = 0; word16 id = 0;
    if (fork() == 0) {
        int rc = SN_Decode_Header(buf, 3, &t, &id);
        fprintf(stderr, "child rc=%d\n", rc);
        _exit(0);
    }
    int s = 0; wait(&s);
    if (WIFSIGNALED(s)) {
        fprintf(stderr, "killed by signal %d\n", WTERMSIG(s));
        return 1;
    }
    return 0;
}

Build and run:

clang -O0 -g -I. -Isrc -DHAVE_CONFIG_H -DWOLFMQTT_SN \
    repro.c src/mqtt_sn_packet.c src/mqtt_packet.c -o repro
./repro

Before this patch: killed by signal 10 (SIGBUS — the read at offset 3
crosses into the guard page).
After this patch: child rc=-3 (MQTT_CODE_ERROR_MALFORMED_DATA), no signal.

Fix

Save the original rx_buf pointer and require that the declared
total_len cover the bytes already consumed plus the upcoming type byte.
The check matches the style of the analogous guard in SN_Decode_Publish
and is confined to SN_Decode_Header; no caller changes are needed.

Build / test

• make clean, no new warnings.
• tests/unit_tests: 173/173 PASS, unchanged from master.
• Reproducer above: pre-patch SIGBUS, post-patch returns
  MQTT_CODE_ERROR_MALFORMED_DATA cleanly.

[wolfSSL-Bot]

Can one of the admins verify this patch?