`nx_packet_data_retrieve()` writes past caller buffer with no bounds check — root-cause stack-buffer overflows in published reference examples

[pbwarmu017]

Environment

• NetX Duo version: 6.4.0 (verified); pattern present in 5.x and 6.x history per git blame
• Latest tagged release at filing: 6.4.3 (Mar 18 2025) — same API shape
• Affected API: nx_packet_data_retrieve() declared in nx_api.h, defined in common/src/nx_packet_data_retrieve.c
• API reference: https://github.com/eclipse-threadx/rtos-docs/blob/main/rtos-docs/netx-duo/chapter4.md

Summary

nx_packet_data_retrieve() writes the entire packet's data into the caller-provided buffer with no buffer_size argument. There is no API-level bound on the write. The only protection available to the caller is hand-checking packet_ptr->nx_packet_length against the buffer's capacity before calling — and that check is omitted in shipping reference examples (filed separately for STMicroelectronics/STM32CubeH5's Nx_TCP_Echo_Server).

When the received packet is larger than the caller's buffer, nx_packet_data_retrieve() overflows the buffer silently and corrupts the surrounding stack frame. The crash signature is precise (HardFault on the corrupted return address, sometimes UFSR.STKOF) but the cause is invisible at the call site without source-level audit upstream.

Reproduction steps

A minimal application thread with a small stack buffer and no nx_packet_length check:

static VOID echo_thread_entry(ULONG arg)
{
    UCHAR buffer[256];
    ULONG bytes_copied;
    NX_PACKET *rx_packet;

    while (1) {
        if (nx_tcp_socket_receive(&socket, &rx_packet, NX_WAIT_FOREVER) != NX_SUCCESS)
            continue;
        nx_packet_data_retrieve(rx_packet, buffer, &bytes_copied);   /* unbounded */
        nx_packet_release(rx_packet);
        ...
    }
}

Send a TCP payload larger than 256 bytes from any host:

import socket
s = socket.socket()
s.connect(("192.168.100.1", 7))
s.sendall(b"X" * 600)

Expected behavior

The API either copies up to a caller-bounded number of bytes and reports truncation, or refuses with a clear error code when the buffer is insufficient.

Actual behavior

nx_packet_data_retrieve() writes 600 bytes starting at buffer, overwriting the surrounding stack frame and the saved return address. Thread either UFSR.STKOFs immediately or HardFaults at the next function epilogue when the corrupted return address is loaded into PC.

SCB->CFSR typically 0x00100000 (UFSR.STKOF) or 0x00008200 (BFSR.PRECISERR | BFARVALID) with BFAR containing overwritten payload bytes. Neither register state hints at the API as the cause.

Root cause

The API signature itself:

UINT nx_packet_data_retrieve(NX_PACKET *packet_ptr, VOID *buffer_start, ULONG *bytes_copied);

bytes_copied is an output parameter (the number of bytes written), not a bound. There is no input parameter that limits the write. The implementation in common/src/nx_packet_data_retrieve.c walks the packet chain and copies the full chain length into buffer_start regardless of the buffer's actual capacity.

packet_ptr->nx_packet_length is available to callers and gives the total payload length, but checking it before the call is not enforced and is the responsibility of every individual caller.

Verification of root cause

Bisecting payload sizes against UCHAR buffer[256]:

Payload size	Result
64 B	OK
256 B	OK
257 B	HardFault (boundary)
300 B	HardFault
600 B	HardFault
1400 B	HardFault

Boundary at exactly the buffer size is the canonical signature of stack-buffer overflow. Confirmed by changing the buffer to UCHAR buffer[1536] — all sizes in the table pass.

Suggested upstream fix

Three options, in order of preferred:

Option 1 — Add a buffer_size parameter (preferred; source-incompatible)

UINT nx_packet_data_retrieve(NX_PACKET *packet_ptr,
                             VOID *buffer_start,
                             ULONG buffer_size,        /* new */
                             ULONG *bytes_copied);

Returns a new NX_BUFFER_TOO_SMALL (or similar) when nx_packet_length > buffer_size. The existing function stays as a deprecated wrapper for one release cycle, emitting __deprecated__ at use; remove in a subsequent major.

Option 2 — Runtime check + clear error code (source-compatible)

If the signature change is out of scope, add a runtime check inside _nx_packet_data_retrieve that returns early with NX_INVALID_PARAMETERS (or new error code) when the implementation cannot determine whether the buffer is sufficient. Combined with __warn_unused_result on the function declaration, this surfaces the issue at debug-build time rather than corrupting the stack silently.

Option 3 — Documentation (minimum)

Update the API reference at rtos-docs/netx-duo/chapter4.md nx_packet_data_retrieve section to add a prominent Warning block:

Warning. The caller-provided buffer must be at least as large as packet_ptr->nx_packet_length. There is no buffer-size argument; the function writes past the end of any smaller buffer and corrupts surrounding memory. Always check packet_ptr->nx_packet_length against the buffer capacity before calling, or pre-size the buffer to the packet pool's payload size.

A documentation warning is a stop-gap, not a fix — it does not stop reference examples from shipping with the bug class — but it is a near-zero-cost mitigation that reduces the rate of new occurrences.

The pattern is broadly distributed across published reference code; an audit of nx_packet_data_retrieve callers in eclipse-threadx repositories and partner firmware packs would surface the scope of existing exposures.

[Hnz2]

You should to use nx_packet_length_get() and nx_packet_data_extract_offset().
Jan

[pbwarmu017]

Thanks Jan — confirmed nx_packet_length_get() + nx_packet_data_extract_offset(..., buffer_size, ...) does everything nx_packet_data_retrieve() does plus the bounds check. I'll move my own examples to that pattern.

The report was less about "how do I write working code today" and more about the API still being on the public surface in a shape that traps readers — the published ST Nx_TCP_Echo_Server reference example calls nx_packet_data_retrieve() directly with an under-sized stack buffer, and that bug class keeps shipping as long as the unsafe API is the path of least resistance for someone reading the API reference cold.

Three possible scopes, happy with whichever makes sense:

1. Deprecate nx_packet_data_retrieve() (attribute((deprecated)) + one-release removal window).
2. Documentation Warning in rtos-docs/netx-duo/chapter4.md pointing readers at nx_packet_data_extract_offset(). Lowest cost.
3. Leave it — totally defensible; in that case I'd ask whether ST's example is something you'd flag downstream.