ASan `stack-buffer-overflow` in real TCP `SendRRData` path when a recognized truncated optional sockaddr CPF item is parsed in `CreateCommonPacketFormatStructure()`

[CarnegieMe]

Summary

I confirmed a remotely reachable out-of-bounds read on current master in the unconnected explicit TCP SendRRData path.

The crash occurs before NotifyMessageRouter() is entered. The root cause is that CreateCommonPacketFormatStructure() recognizes optional sockaddr info items (0x8000 / 0x8001) and then unconditionally reads the rest of the fixed sockaddr structure without first verifying that those bytes are still within the received CPF buffer. The first invalid access occurs in GetIntFromMessage(), and ASan reports it as a stack-buffer-overflow because the out-of-bounds read crosses the right boundary of incoming_message[512] in HandleDataOnTcpSocket().

Version

• OpENer v2.3 / master branch up to commit 76b95cf

Impact

A remote client can establish a normal TCP session on port 44818, send RegisterSession, and then trigger a crash with a single malformed SendRRData packet. Under ASan, this is a stable stack-buffer-overflow read. In a non-ASan build, this is at least a remotely reachable denial of service and causes the parser to interpret bytes beyond the intended CPF boundary as sockaddr fields.

Affected path

The real execution path is:

main()
  -> executeEventLoop()
  -> NetworkHandlerProcessCyclic()
  -> HandleDataOnTcpSocket()
  -> HandleReceivedExplictTcpData()
  -> HandleReceivedSendRequestResponseDataCommand()
  -> NotifyCommonPacketFormat()
  -> CreateCommonPacketFormatStructure()
  -> GetIntFromMessage()

This matches the ASan backtrace from the unmodified ASan/UBSan build and also matches the current master source routing for TCP explicit messages and CPF parsing.

Reproduction

A minimal reproducer is:

1. Start the unmodified OpENer POSIX server compiled with ASan/UBSan.
2. Open a real TCP connection to 44818.
3. Send RegisterSession.
4. Send a SendRRData packet whose CPF:

• sets item_count = 3,
• contains a valid Null Address Item,
• contains a large Unconnected Data Item whose declared length still fits in the received CPF length checks,
• places a recognized optional sockaddr type id (0x8000) in the final 2 bytes of the packet.

Build

cmake -S source -B build-asan \
    -DCMAKE_C_COMPILER=clang \
    -DCMAKE_CXX_COMPILER=clang++ \
    -DOpENer_PLATFORM:STRING=POSIX \
    -DCMAKE_BUILD_TYPE:STRING=RelWithDebInfo \
    -DBUILD_SHARED_LIBS:BOOL=OFF \
    -DOpENer_TRACES:BOOL=ON \
    -DOpENer_TRACE_LEVEL_ERROR:BOOL=ON \
    -DCMAKE_C_FLAGS:STRING="-O1 -g -fno-omit-frame-pointer -fno-optimize-sibling-calls -fsanitize=address,undefined" \
    -DCMAKE_CXX_FLAGS:STRING="-O1 -g -fno-omit-frame-pointer -fno-optimize-sibling-calls -fsanitize=address,undefined" \
    -DCMAKE_EXE_LINKER_FLAGS:STRING="-fsanitize=address,undefined -pthread"

  cmake --build build-asan --target OpENer -j"$(nproc)"

Start server

  ./build-asan/src/ports/POSIX/OpENer ens33

Poc

poc.zip

Baseline

  python3 poc.py baseline 192.168.153.128

outputs:

mode=baseline host=192.168.153.128 payload_len=24 packet_len=48
session=1
reply_service=0x8e general_status=0x00 additional_status=[] payload=0100

Crash

  python3 poc.py crash 192.168.153.128

outputs:

mode=crash host=192.168.153.128 payload_len=488 packet_len=512
session=1
response=timeout

The baseline mode successfully returns a normal GetAttributeSingle reply, while the crash mode produces a timeout on the client and an ASan stack-buffer-overflow on the server. The server log shows Sockaddr type id: 8000 immediately before the crash, which is consistent with the parser successfully reading the optional item type and then failing on the next field read.

Root Cause

1. The server reads the full TCP ENIP message into a stack buffer

HandleDataOnTcpSocket() stores the received TCP message in stack memory (incoming_message[...]). ASan reports that the invalid read crosses the right boundary of that stack buffer. This is why the failure is classified as stack-buffer-overflow rather than heap OOB.

Relevant logic, simplified:

void HandleDataOnTcpSocket(...) {
    EipUint8 incoming_message[PC_OPENER_ETHERNET_BUFFER_SIZE];
    ...
    recv(socket, incoming_message, ...);
    HandleReceivedExplictTcpData(&incoming_message[0], ...);
}

2. SendRRData reaches CPF parsing on the real server path

HandleReceivedSendRequestResponseDataCommand() forwards SendRRData traffic to NotifyCommonPacketFormat(), which then calls CreateCommonPacketFormatStructure() on the received CPF buffer. This is the exact parser reached in the crashing backtrace.

Relevant logic, simplified:

EipStatus HandleReceivedSendRequestResponseDataCommand(...) {
    return NotifyCommonPacketFormat(received_data, originator_address, outgoing_message);
}

3. CreateCommonPacketFormatStructure() validates the main data item, but not the recognized optional sockaddr item body

The function first parses the mandatory address item and data item. For the data item, it checks that:

data_length >= length_count + 4 + data_item.length

and only then advances data to the end of the data item. That check is real, but it only proves that the main data item fits. It does not prove that later optional items fit.

Then the function enters the optional item loop. Current master still contains the following logic pattern, together with the in-source comment indicating a missing bound check.

Relevant logic, simplified from current master:

address_item_count = item_count - 2;

for (j = 0; j < min(address_item_count, 2); j++) {   // TODO there needs to be a limit check here???
    type_id = GetIntFromMessage(&data);
    length_count += 2;

    if (type_id == 0x8000 || type_id == 0x8001) {
        length     = GetIntFromMessage(&data);
        sin_family = GetIntFromMessage(&data);
        sin_port   = GetIntFromMessage(&data);
        sin_addr   = GetUdintFromMessage(&data);

        for (i = 0; i < 8; i++) {
            nasin_zero[i] = *data;
            data++;
        }

        length_count += 18;
    } else {
        type_id = 0;
        data -= 2;
    }
}

The bug is that once the parser recognizes 0x8000 / 0x8001, it immediately consumes the rest of the fixed sockaddr structure without first checking that the remaining CPF bytes are sufficient for:

• length (2)
• sin_family (2)
• sin_port (2)
• sin_addr (4)
• nasin_zero[8] (8)

That is a fixed 18-byte read after the recognized type id path is taken.

4. GetIntFromMessage() is a raw advancing read primitive with no bounds checking

The first crashing primitive is GetIntFromMessage(). In current master, this helper reads two bytes from the current pointer and advances the pointer; it does not receive a buffer end pointer and does not perform any independent length validation. So once the caller miscomputes “remaining bytes”, the helper directly dereferences out of bounds.

Relevant logic, simplified:

EipUint16 GetIntFromMessage(const EipUint8 **buffer) {
    value = (*buffer)[0] | ((*buffer)[1] << 8);
    *buffer += 2;
    return value;
}

5. Why the PoC is reliable

The PoC places a recognized optional sockaddr type id (0x8000) in the final 2 bytes of the packet. This makes the first GetIntFromMessage(&data) still succeed, so the server prints:

Sockaddr type id: 8000

Immediately after that, the parser tries to read the next field (length) using another GetIntFromMessage(&data), but data already points at the end of the TCP receive buffer. ASan then reports the first invalid access at endianconv.c:68 and shows that the read crosses the right edge of incoming_message in HandleDataOnTcpSocket().

Why this is not just a generic malformed-length issue

This is not merely “CPF length mismatch accepted too late”.

The first invalid access occurs before the function reaches its final length_count == data_length consistency check. In other words, the parser does not first detect that the optional item is incomplete and then mishandle the error; instead, it performs out-of-bounds reads while trying to parse a recognized optional sockaddr item and only has the late length reconciliation logic afterward. The vulnerability is therefore specifically a pre-validation out-of-bounds read in the recognized optional sockaddr branch.

ASan evidence

Observed server-side crash:

=================================================================
==91221==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffffa35dcd0 at pc 0x55cf95791217 bp 0x7ffffa35d3d0 sp 0x7ffffa35d3c8
READ of size 1 at 0x7ffffa35dcd0 thread T0
    #0 0x55cf95791216 in GetIntFromMessage /home/weichuan/wc/OpENer/source/src/enet_encap/endianconv.c:68:20
    #1 0x55cf957879ca in CreateCommonPacketFormatStructure /home/weichuan/wc/OpENer/source/src/enet_encap/cpf.c:290:11
    #2 0x55cf95786ace in NotifyCommonPacketFormat /home/weichuan/wc/OpENer/source/src/enet_encap/cpf.c:46:12
    #3 0x55cf9578df60 in HandleReceivedSendRequestResponseDataCommand /home/weichuan/wc/OpENer/source/src/enet_encap/encap.c:558:22
    #4 0x55cf9578ca83 in HandleReceivedExplictTcpData /home/weichuan/wc/OpENer/source/src/enet_encap/encap.c:186:26
    #5 0x55cf9574d554 in HandleDataOnTcpSocket /home/weichuan/wc/OpENer/source/src/ports/generic_networkhandler.c:864:30
    #6 0x55cf9574b9c1 in NetworkHandlerProcessCyclic /home/weichuan/wc/OpENer/source/src/ports/generic_networkhandler.c:497:32
    #7 0x55cf957491a4 in executeEventLoop /home/weichuan/wc/OpENer/source/src/ports/POSIX/main.c:261:24
    #8 0x55cf957491a4 in main /home/weichuan/wc/OpENer/source/src/ports/POSIX/main.c:229:12
    #9 0x7175e5229d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #10 0x7175e5229e3f in __libc_start_main csu/../csu/libc-start.c:392:3
    #11 0x55cf9568b414 in _start (/home/weichuan/wc/OpENer/build-asan/src/ports/POSIX/OpENer+0x4f414) (BuildId: 00508863225e3b150de78c4df03f21303a3fa43e)

Address 0x7ffffa35dcd0 is located in stack of thread T0 at offset 560 in frame
    #0 0x55cf9574d1ff in HandleDataOnTcpSocket /home/weichuan/wc/OpENer/source/src/ports/generic_networkhandler.c:720

  This frame has 6 object(s):
    [32, 36) 'remaining_bytes' (line 722)
    [48, 560) 'incoming_message' (line 732) <== Memory access at offset 560 overflows this variable
    [624, 632) 'read_buffer' (line 760)
    [656, 672) 'sender_address' (line 850)
    [688, 692) 'fromlen' (line 852)
    [704, 1232) 'outgoing_message' (line 862)
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow /home/weichuan/wc/OpENer/source/src/enet_encap/endianconv.c:68:20 in GetIntFromMessage
Shadow bytes around the buggy address:
  0x10007f463b40: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007f463b50: 00 00 00 00 f1 f1 f1 f1 04 f2 00 00 00 00 00 00
  0x10007f463b60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007f463b70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007f463b80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x10007f463b90: 00 00 00 00 00 00 00 00 00 00[f2]f2 f2 f2 f2 f2
  0x10007f463ba0: f2 f2 00 f2 f2 f2 00 00 f2 f2 04 f2 00 00 00 00
  0x10007f463bb0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007f463bc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007f463bd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007f463be0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 f3 f3
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==91221==ABORTING

This was reproduced on the unmodified server with only a client-side PoC added. The baseline request also succeeds first on the same real path, returning a normal GetAttributeSingle reply.

Suggested fix

The minimal fix is to make optional item parsing length-driven before field consumption, not type-driven after reading into the structure.

Recommended fix strategy

1. Before reading any optional item type_id, verify that at least 2 bytes remain.
2. After reading type_id, if it is 0x8000 or 0x8001, verify that at least the full encoded sockaddr payload remains before reading any further fields.
3. Reject recognized sockaddr items whose declared length is not exactly 16.
4. Return kEipStatusError immediately on truncation instead of deferring to the final length_count == data_length reconciliation.

Example patch direction

size_t remaining = 0;

for (size_t j = 0; j < (address_item_count > 2 ? 2 : address_item_count); j++) {
    remaining = data_length - length_count;
    if (remaining < 2) {
        return kEipStatusError;
    }

    type_id = GetIntFromMessage(&data);
    length_count += 2;

    if (type_id == kCipItemIdSocketAddressInfoOriginatorToTarget ||
        type_id == kCipItemIdSocketAddressInfoTargetToOriginator) {

        /* length(2) + family(2) + port(2) + addr(4) + zero[8] = 18 bytes after type */
        remaining = data_length - length_count;
        if (remaining < 18) {
            return kEipStatusError;
        }

        length = GetIntFromMessage(&data);
        if (length != 16) {
            return kEipStatusError;
        }

        sin_family = GetIntFromMessage(&data);
        sin_port   = GetIntFromMessage(&data);
        sin_addr   = GetUdintFromMessage(&data);

        for (size_t i = 0; i < 8; i++) {
            nasin_zero[i] = *data++;
        }

        length_count += 18;
    } else {
        return kEipStatusError; /* or skip only with a validated generic item header */
    }
}