Cross-protocol stale CPF `data_item` reuse causes `stack-use-after-return` in TCP SendUnitData path

## Summary

A remotely reachable `stack-use-after-return` exists in the current OpENer `master` branch in the EtherNet/IP Common Packet Format (CPF) handling logic.

The issue is caused by reusing the process-global CPF parsing state `g_common_packet_format_data_item` across different messages and protocol paths. `CreateCommonPacketFormatStructure()` only refreshes `data_item` when `item_count >= 2`. When a later CPF packet has `item_count == 1`, the function updates the address item but leaves the previous `data_item.type_id`, `data_item.length`, and `data_item.data` unchanged.

As a result, a valid UDP I/O consuming packet can first populate `g_common_packet_format_data_item.data_item.data` with a pointer into the stack buffer of `CheckAndHandleConsumingUdpSocket()`. A later TCP `SendUnitData` packet with only a `ConnectionAddress` item (`item_count = 1`) can then reuse that stale UDP `data_item` in the TCP connected explicit path, causing `NotifyConnectedCommonPacketFormat()` to read from a returned UDP stack frame.

This is reproducible against the unmodified POSIX OpENer server using a client-side PoC.

## Version

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

## Affected component

- EtherNet/IP Common Packet Format parsing
- Connected explicit messaging path
- UDP I/O consuming path
- TCP `SendUnitData` path

Relevant source files:

- `source/src/enet_encap/cpf.c`
- `source/src/enet_encap/encap.c`
- `source/src/ports/generic_networkhandler.c`
- `source/src/enet_encap/endianconv.c`

## Impact

A remote attacker who can reach the OpENer EtherNet/IP service can establish normal CIP connections and send a crafted sequence of TCP and UDP packets that causes the server to dereference a pointer into a returned UDP stack frame.

In ASan builds, this is reported as:

```text
AddressSanitizer: stack-use-after-return
```

At minimum, this is a remotely triggerable `denial of service`. In non-ASan builds, the stale stack pointer may cause process crash, protocol-state confusion, or consumption of stale stack bytes as connected explicit message data.

## Technical details

### 1. CPF parsing state is global

`cpf.c` stores CPF parsing results in a global object:

```c
CipCommonPacketFormatData g_common_packet_format_data_item; /**< CPF global data items */
```

This same global object is used by multiple paths, including:

- TCP unconnected explicit path
- TCP connected explicit path
- UDP I/O consuming path

For example, the TCP connected explicit path calls:

```c
EipStatus return_value = CreateCommonPacketFormatStructure(
  received_data->current_communication_buffer_position,
  received_data->data_length,
  &g_common_packet_format_data_item);
```

The UDP I/O consuming path also eventually parses into the same global object:

```c
CreateCommonPacketFormatStructure(data, data_length,
                                  &g_common_packet_format_data_item)
```

Therefore, CPF data from different messages and different transport paths share the same storage.

### 2. `CreateCommonPacketFormatStructure()` does not reset `data_item`

At the start of `CreateCommonPacketFormatStructure()`, only `address_info_item` is partially reset:

```c
common_packet_format_data->address_info_item[0].type_id = 0;
common_packet_format_data->address_info_item[1].type_id = 0;
```

The function then parses the address item when `item_count >= 1`:

```c
CipUint item_count = GetUintFromMessage(&data);
common_packet_format_data->item_count = item_count;
length_count += 2;

if(common_packet_format_data->item_count >= 1U) {
  common_packet_format_data->address_item.type_id = GetUintFromMessage(&data);
  common_packet_format_data->address_item.length = GetUintFromMessage(&data);
  length_count += 4;

  if(common_packet_format_data->address_item.length >= 4) {
    common_packet_format_data->address_item.data.connection_identifier =
      GetUdintFromMessage(&data);
    length_count += 4;
  }

  if(common_packet_format_data->address_item.length == 8) {
    common_packet_format_data->address_item.data.sequence_number =
      GetUdintFromMessage(&data);
    length_count += 4;
  }
}
```

However, the data item is only parsed when `item_count >= 2`:

```c
if(common_packet_format_data->item_count >= 2) {
  common_packet_format_data->data_item.type_id = GetUintFromMessage(&data);
  common_packet_format_data->data_item.length = GetUintFromMessage(&data);
  common_packet_format_data->data_item.data = (EipUint8 *) data;

  if(data_length >=
     length_count + 4 + common_packet_format_data->data_item.length) {
    data += common_packet_format_data->data_item.length;
    length_count += (4 + common_packet_format_data->data_item.length);
  } else {
    return kEipStatusError;
  }

  ...
}
```

When `item_count == 1`, the function updates `item_count` and `address_item`, but it does not clear or invalidate:

```c
common_packet_format_data->data_item.type_id
common_packet_format_data->data_item.length
common_packet_format_data->data_item.data
```

Because the target structure is global, these fields retain values from a previous message.

### 3. UDP I/O consuming path can leave a stack pointer in `data_item.data`

The UDP I/O consuming path receives data into a local stack buffer:

```c
void CheckAndHandleConsumingUdpSocket(void) {
  ...
  CipOctet incoming_message[PC_OPENER_ETHERNET_BUFFER_SIZE] = { 0 };

  int received_size = recvfrom(g_network_status.udp_io_messaging,
                               NWBUF_CAST incoming_message,
                               sizeof(incoming_message),
                               0,
                               (struct sockaddr *) &from_address,
                               &from_address_length);
  ...
  HandleReceivedConnectedData(incoming_message, received_size,
                              &from_address);
}
```

`HandleReceivedConnectedData()` parses this UDP packet into the global CPF object:

```c
CreateCommonPacketFormatStructure(data, data_length,
                                  &g_common_packet_format_data_item)
```

For a normal connected UDP CPF with `item_count = 2`, `CreateCommonPacketFormatStructure()` stores the data item pointer as:

```c
common_packet_format_data->data_item.data = (EipUint8 *) data;
```

At this point, `data` points inside the UDP stack buffer `incoming_message`.

When `CheckAndHandleConsumingUdpSocket()` returns, this stack frame is no longer valid, but the global `g_common_packet_format_data_item.data_item.data` still points into it.

### 4. TCP `SendUnitData(item_count=1)` consumes the stale UDP data item

The TCP path receives an encapsulated packet in `HandleDataOnTcpSocket()` and dispatches it through:

```c
HandleReceivedExplictTcpData(...)
```

For `SendUnitData`, the code reaches:

```c
HandleReceivedSendUnitDataCommand(...)
```

That function skips the command-specific fields and calls the connected CPF handler:

```c
GetDintFromMessage((const EipUint8** const ) &receive_data->current_communication_buffer_position);
GetIntFromMessage((const EipUint8** const ) &receive_data->current_communication_buffer_position);
((EncapsulationData* const ) receive_data)->data_length -= 6;

if(kSessionStatusValid == CheckRegisteredSessions(receive_data)) {
  return_value = NotifyConnectedCommonPacketFormat(receive_data,
                                                   originator_address,
                                                   outgoing_message);
}
```

Inside `NotifyConnectedCommonPacketFormat()`, the current TCP CPF is parsed into the same global object:

```c
EipStatus return_value = CreateCommonPacketFormatStructure(
  received_data->current_communication_buffer_position,
  received_data->data_length,
  &g_common_packet_format_data_item);
```

If the TCP CPF has `item_count = 1` and contains only a `ConnectionAddress` item, the current TCP packet refreshes only the address item. The stale UDP `data_item` is left unchanged.

The handler then checks the current address item:

```c
if(g_common_packet_format_data_item.address_item.type_id ==
   kCipItemIdConnectionAddress)
```

This succeeds because the TCP packet provides a valid `ConnectionAddress` item and a valid class3 connection ID.

Then the handler checks the data item:

```c
if(g_common_packet_format_data_item.data_item.type_id ==
   kCipItemIdConnectedDataItem)
```

This may also succeed, but the value is stale from the earlier UDP seed packet, not from the current TCP packet.

The stale pointer is then consumed:

```c
EipUint8 *buffer = g_common_packet_format_data_item.data_item.data;
g_common_packet_format_data_item.address_item.data.sequence_number =
  GetUintFromMessage( (const EipUint8 **const ) &buffer );
```

At this point, `buffer` points into the returned stack frame of `CheckAndHandleConsumingUdpSocket()`, causing stack-use-after-return.

### 5. Actual crashing read

The final read occurs in `GetUintFromMessage()`:

```c
CipUint GetUintFromMessage(const CipOctet **const buffer_address) {
  const CipOctet *buffer = *buffer_address;
  EipUint16 data = buffer[0] | buffer[1] << 8;
  *buffer_address += 2;
  return data;
}
```

This function is not the root cause by itself. It is the crash site because it receives a stale pointer from `NotifyConnectedCommonPacketFormat()`.

## Trigger conditions

The reproduced trigger sequence is:

**1.** Establish a TCP EtherNet/IP session with `RegisterSession`.
**2.** Send a valid `ForwardOpen` for an I/O connection.
**3.** Send a valid `ForwardOpen` for a class3 connected explicit connection.
**4.** Send one normal class3 `SendUnitData` request to prime the class3 sequence state.
**5.** Send a UDP connected I/O packet with `item_count = 2`, causing `g_common_packet_format_data_item.data_item.data` to point into the UDP stack buffer.
**6.** Send a malformed TCP `SendUnitData` packet with `item_count = 1` and only a `ConnectionAddress` item.
**7.** The TCP connected explicit path reuses the stale UDP `data_item.data` and reads from the returned UDP stack frame.

The final TCP packet is malformed because it omits the connected data item. The vulnerability is that OpENer reuses the previous message's `data_item` instead of rejecting the packet or clearing the stale state.

## Reproduction

### 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)"
```

### Run server

```bash
ASAN_OPTIONS=detect_stack_use_after_return=1 ./build-asan/src/ports/POSIX/OpENer ens33
```

### Poc

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

### Run client PoC

```bash
python3 poc.py udp_to_tcp 192.168.153.128 494
```

The PoC performs:

- `RegisterSession`
- `ForwardOpen` for an I/O connection
- `ForwardOpen` for a class3 connected explicit connection
- A normal class3 `SendUnitData` request
- A UDP connected seed packet
- A malformed TCP `SendUnitData(item_count=1)` packet

### Expected client output

Example output:

```bash
session=1
forward_open tag=IOFOOPEN consumed=0x7b510013 produced=0xabc00002
forward_open tag=C3FOOPEN consumed=0x7b510014 produced=0xabc10002
prime_response_len=52
udp_seed packet_len=512 io_connection_id=0x7b510013 eip_seq=7 data_len=494
stale_tcp_response_len=0
```

### Expected server crash

The server crashes with ASan:

```bash
=================================================================
==108360==ERROR: AddressSanitizer: stack-use-after-return on address 0x75db24302c62 at pc 0x5b17bacd93c7 bp 0x7ffd7c227350 sp 0x7ffd7c227348
READ of size 1 at 0x75db24302c62 thread T0
    #0 0x5b17bacd93c6 in GetUintFromMessage /home/weichuan/wc/OpENer/source/src/enet_encap/endianconv.c:75:20
    #1 0x5b17bacd1fa8 in NotifyConnectedCommonPacketFormat /home/weichuan/wc/OpENer/source/src/enet_encap/cpf.c:141:13
    #2 0x5b17bacd6190 in HandleReceivedSendUnitDataCommand /home/weichuan/wc/OpENer/source/src/enet_encap/encap.c:526:22
    #3 0x5b17bacd49ef in HandleReceivedExplictTcpData /home/weichuan/wc/OpENer/source/src/enet_encap/encap.c:191:26
    #4 0x5b17bac95554 in HandleDataOnTcpSocket /home/weichuan/wc/OpENer/source/src/ports/generic_networkhandler.c:864:30
    #5 0x5b17bac939c1 in NetworkHandlerProcessCyclic /home/weichuan/wc/OpENer/source/src/ports/generic_networkhandler.c:497:32
    #6 0x5b17bac911a4 in executeEventLoop /home/weichuan/wc/OpENer/source/src/ports/POSIX/main.c:261:24
    #7 0x5b17bac911a4 in main /home/weichuan/wc/OpENer/source/src/ports/POSIX/main.c:229:12
```

ASan also identifies the stale address as belonging to the previous UDP receiving stack frame:

```bash
Address 0x75db24302c62 is located in stack of thread T0 at offset 98 in frame
    #0 0x5b17bac94b2f in CheckAndHandleConsumingUdpSocket /home/weichuan/wc/OpENer/source/src/ports/generic_networkhandler.c:1050

  This frame has 3 object(s):
    [32, 48) 'from_address' (line 1067)
    [64, 68) 'from_address_length' (line 1068)
    [80, 592) 'incoming_message' (line 1069) <== Memory access at offset 98 is inside this variable
```

The offset is significant:

```bash
98 - 80 = 18
```

For a connected CPF packet, offset 18 is exactly the beginning of the connected data item payload:

```bash
0x00: item_count                  2 bytes
0x02: address_item.type_id        2 bytes
0x04: address_item.length         2 bytes
0x06: connection_id               4 bytes
0x0a: sequence_number             4 bytes
0x0e: data_item.type_id           2 bytes
0x10: data_item.length            2 bytes
0x12: data_item.data              <-- offset 18
```

This confirms that the TCP connected explicit path is reading a stale `data_item.data` pointer left by the previous UDP connected packet.

## Why this is distinct from related CPF issues

This issue is not the same as previously reported CPF `item_count` overflow or out-of-bounds parsing issues.

The trigger here does not require an oversized `item_count`. The final TCP packet uses `item_count = 1`.

The bug is not caused by parsing too many CPF items. Instead, it is caused by parsing too few CPF items while leaving the old `data_item` valid-looking in a global CPF object.

This issue is also distinct from TCP-only stack-use-after-return reports. In this case, ASan confirms that the stale address comes from the UDP `CheckAndHandleConsumingUdpSocket()` stack frame, not from the current TCP `HandleDataOnTcpSocket()` stack frame.

## Root cause

The root cause is the combination of:

**1.** Using a global CPF parsing object across messages and transport paths:

```c
CipCommonPacketFormatData g_common_packet_format_data_item;
```

**2.** Incomplete initialization in CreateCommonPacketFormatStructure():

```c
common_packet_format_data->address_info_item[0].type_id = 0;
common_packet_format_data->address_info_item[1].type_id = 0;
```

**3.** Conditional refresh of `data_item` only when `item_count >= 2`:

```bash
if(common_packet_format_data->item_count >= 2) {
  common_packet_format_data->data_item.type_id = GetUintFromMessage(&data);
  common_packet_format_data->data_item.length = GetUintFromMessage(&data);
  common_packet_format_data->data_item.data = (EipUint8 *) data;
  ...
}
```

**4.** `NotifyConnectedCommonPacketFormat()` consuming `data_item` without verifying that the current CPF actually contained a data item:

```c
if(g_common_packet_format_data_item.data_item.type_id ==
   kCipItemIdConnectedDataItem) {
  EipUint8 *buffer = g_common_packet_format_data_item.data_item.data;
  g_common_packet_format_data_item.address_item.data.sequence_number =
    GetUintFromMessage( (const EipUint8 **const ) &buffer );
  ...
}
```

## Suggested fix

### Minimal fix

Fully initialize the CPF parsing result at the beginning of `CreateCommonPacketFormatStructure()`.

For example:

```c
EipStatus CreateCommonPacketFormatStructure(const EipUint8 *data,
                                            size_t data_length,
                                            CipCommonPacketFormatData *common_packet_format_data)
{
  if(NULL == common_packet_format_data) {
    return kEipStatusError;
  }

  memset(common_packet_format_data, 0, sizeof(*common_packet_format_data));

  ...
}
```

If clearing the whole structure is considered too broad, at least clear `address_item`, `data_item`, and `address_info_item` before parsing:

```c
memset(&common_packet_format_data->address_item,
       0,
       sizeof(common_packet_format_data->address_item));

memset(&common_packet_format_data->data_item,
       0,
       sizeof(common_packet_format_data->data_item));

memset(&common_packet_format_data->address_info_item,
       0,
       sizeof(common_packet_format_data->address_info_item));
```

### Required validation in connected CPF path

`NotifyConnectedCommonPacketFormat()` should reject connected messages unless the current CPF actually contains both a connection address item and a connected data item.

For example:

```c
if(g_common_packet_format_data_item.item_count < 2) {
  OPENER_TRACE_ERR("notifyConnectedCPF: missing connected data item\n");
  return kEipStatusError;
}

if(g_common_packet_format_data_item.address_item.type_id !=
   kCipItemIdConnectionAddress) {
  OPENER_TRACE_ERR("notifyConnectedCPF: missing connection address item\n");
  return kEipStatusError;
}

if(g_common_packet_format_data_item.data_item.type_id !=
   kCipItemIdConnectedDataItem) {
  OPENER_TRACE_ERR("notifyConnectedCPF: missing connected data item\n");
  return kEipStatusError;
}

if(NULL == g_common_packet_format_data_item.data_item.data ||
   g_common_packet_format_data_item.data_item.length < 2) {
  OPENER_TRACE_ERR("notifyConnectedCPF: invalid connected data item\n");
  return kEipStatusError;
}
```

This prevents `item_count = 1` packets from reusing stale `data_item` state.

### Structural fix

Avoid using `g_common_packet_format_data_item` as cross-message global mutable state. CPF parsing results should be local to the current packet and passed explicitly to subsequent handlers.

For example:

```c
EipStatus NotifyConnectedCommonPacketFormat(
  const EncapsulationData *const received_data,
  const struct sockaddr *const originator_address,
  ENIPMessage *const outgoing_message)
{
  CipCommonPacketFormatData cpf;
  memset(&cpf, 0, sizeof(cpf));

  EipStatus return_value = CreateCommonPacketFormatStructure(
    received_data->current_communication_buffer_position,
    received_data->data_length,
    &cpf);

  if(kEipStatusError == return_value) {
    return kEipStatusError;
  }

  if(cpf.item_count < 2 ||
     cpf.address_item.type_id != kCipItemIdConnectionAddress ||
     cpf.data_item.type_id != kCipItemIdConnectedDataItem ||
     cpf.data_item.data == NULL ||
     cpf.data_item.length < 2) {
    return kEipStatusError;
  }

  ...
}
```

The same principle should be applied to UDP and unconnected CPF paths so that one packet cannot leave parser state that affects a later packet.


Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cross-protocol stale CPF `data_item` reuse causes `stack-use-after-return` in TCP SendUnitData path #575

Summary

Version

Affected component

Impact

Technical details

1. CPF parsing state is global

2. `CreateCommonPacketFormatStructure()` does not reset `data_item`

3. UDP I/O consuming path can leave a stack pointer in `data_item.data`

4. TCP `SendUnitData(item_count=1)` consumes the stale UDP data item

5. Actual crashing read

Trigger conditions

Reproduction

Build

Run server

Poc

Run client PoC

Expected client output

Expected server crash

Why this is distinct from related CPF issues

Root cause

Suggested fix

Minimal fix

Required validation in connected CPF path

Structural fix

Metadata

Assignees

Labels

Type

Fields

Projects

Milestone

Relationships

Development

Cross-protocol stale CPF data_item reuse causes stack-use-after-return in TCP SendUnitData path #575

Description

Summary

Version

Affected component

Impact

Technical details

1. CPF parsing state is global

2. CreateCommonPacketFormatStructure() does not reset data_item

3. UDP I/O consuming path can leave a stack pointer in data_item.data

4. TCP SendUnitData(item_count=1) consumes the stale UDP data item

5. Actual crashing read

Trigger conditions

Reproduction

Build

Run server

Poc

Run client PoC

Expected client output

Expected server crash

Why this is distinct from related CPF issues

Root cause

Suggested fix

Minimal fix

Required validation in connected CPF path

Structural fix

Metadata

Metadata

Assignees

Labels

Type

Fields

Projects

Milestone

Relationships

Development

Issue actions

Cross-protocol stale CPF `data_item` reuse causes `stack-use-after-return` in TCP SendUnitData path #575

2. `CreateCommonPacketFormatStructure()` does not reset `data_item`

3. UDP I/O consuming path can leave a stack pointer in `data_item.data`

4. TCP `SendUnitData(item_count=1)` consumes the stale UDP data item