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

## 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

```bash
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

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

### Poc

[poc.zip](https://github.com/user-attachments/files/28331712/poc.zip)

### Baseline

```bash
python3 poc.py baseline 192.168.153.128
```

outputs:

```bash
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

```bash
python3 poc.py crash 192.168.153.128
```

outputs:

```bash
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:

```c
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:

```c
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:

```c
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`:

```c
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:

```c
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:

```c
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:

```bash
=================================================================
==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

```c
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 */
}
}
```