Heap buffer overflow in mbus_frame_internal_pack() (mbus-protocol.c)

[akoul]

Summary

libmbus (all versions) has a heap buffer overflow in mbus_frame_internal_pack() (mbus-protocol.c, lines 4240-4266): writes records into frame->data[252] with zero bounds checking, overflowing by 16+ bytes into adjacent struct fields.

Confirmed with AddressSanitizer. Distinct from issue #223 (which covers the manufacturer-specific data path in mbus_data_variable_parse()).

---

Root cause

// mbus-protocol.c, lines 4240-4266
for (record = data->record; record; record = record->next)
{
    frame->data[frame->data_size++] = record->drh.dib.dif;      // no bounds check
    for (j = 0; j < record->drh.dib.ndife; j++)
        frame->data[frame->data_size++] = record->drh.dib.dife[j]; // no bounds check
    frame->data[frame->data_size++] = record->drh.vib.vif;      // no bounds check
    for (j = 0; j < record->drh.vib.nvife; j++)
        frame->data[frame->data_size++] = record->drh.vib.vife[j]; // no bounds check
    for (j = 0; j < record->data_len; j++)
        frame->data[frame->data_size++] = record->data[j];      // no bounds check
}

A single record with max extensions (ndife=10, nvife=10, data_len=234) writes 256+ bytes into data[252], overflowing into data_size, type, timestamp, and potentially next pointer.

ASan output

ERROR: AddressSanitizer: heap-buffer-overflow on address 0x61200000030a
WRITE of size 1 at 0x61200000030a thread T0
    #0 in mbus_frame_internal_pack mbus-protocol.c:4265
0x61200000030a is located 34 bytes after 296-byte region

Impact

Heap corruption via re-serialization workflow (parse → modify → pack). Overwrites struct fields including linked-list pointers. Attack surface: smart meters, building automation, SCADA/ICS environments using M-Bus over TCP/IP or serial gateways.

Suggested fix

if (frame->data_size >= MBUS_FRAME_DATA_LENGTH)
    return -1;
frame->data[frame->data_size++] = ...;

Coordinated disclosure

I am concurrently requesting a CVE ID from MITRE. Bug 2 (off-by-one in custom VIF decode) has been moved to a separate issue per maintainer request.

[lategoodbye]

@akoul Thank you for the report. Could you please split this report?

[akoul]

would you like me to file two separate issues and close this one out?

[lategoodbye]

No, you can keep this issue for bug 1 and cut the second out and move it to a new issue. It makes it complicated to have multiple issues under a single id.

In case you contact Robert before, he doesn't maintain this project anymore. So I added a security policy yesterday. Since the vulnerability is public it's better to keep it here.

[akoul]

Got it — I'll keep this issue for Bug 1 (heap buffer overflow in mbus_frame_internal_pack) and move Bug 2 to a new issue. Thanks for the heads up about the maintainer change and the security policy.