EIPStackGroup / EIPStackGroup/OpENer

Remote `stack-buffer-overflow read` in Connection Manager services after long padded EPath leaves zero request body

Open
#577 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
C
Stars
857
Forks
314
Avg merge
18d 2h
Merged PRs (30d)
1

Description

## Summary

A crafted EtherNet/IP `SendRRData` request can reach the Connection Manager on the current `master` branch and trigger a remote **stack-buffer-overflow read** in one of three instance services:

- `ForwardClose` (`0x4E`)
- `GetConnectionData` (`0x56`)
- `SearchConnectionData` (`0x57`)

The root cause is **missing minimum request-body length validation in these service handlers** after Message Router path decoding. A long but syntactically acceptable padded EPath can consume the entire CIP payload, leaving `message_router_request->request_data_size == 0`, yet the handlers still directly call `GetUintFromMessage()` / `GetUdintFromMessage()` on `message_router_request->data`.

This is a **real network-reachable server-side bug** in the TCP explicit messaging path (`RegisterSession` -> `SendRRData`) and is reproducible without `ForwardOpen` or special build options.

At minimum, this is a **remote unauthenticated denial of service**.

## Version

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

## Impact

A remote attacker who can reach the EtherNet/IP TCP service (default port `44818`) can send a crafted explicit message that causes the server to read past the end of the stack receive buffer and abort under ASan.

The currently verified impact is:

- **remote stack out-of-bounds read**
- **server process crash / DoS**

## Why this is one vulnerability family, not three unrelated bugs

The three PoC modes differ only in the CIP service code:

- `0x4E` -> `ForwardClose`
- `0x56` -> `GetConnectionData`
- `0x57` -> `SearchConnectionData`

The packet structure, transport path, padded EPath technique, and final read primitive are otherwise the same.

This shows a **shared root cause**:

**1.** a long padded EPath is accepted,
**2.** Message Router leaves `request_data_size == 0`,
**3.** multiple Connection Manager handlers fail to validate their required minimum body length,
**4.** all of them end up reading from the same stack-backed TCP receive buffer tail.

So this should be treated as **one Connection Manager short-request / padded-path vulnerability family** affecting multiple handlers, not as three separate unrelated issues.

## Reachable network path

The bug is reachable through the real server TCP path:

```c
main()
-> executeEventLoop()
-> NetworkHandlerProcessCyclic()
-> HandleDataOnTcpSocket()
-> HandleReceivedExplictTcpData()
-> HandleReceivedSendRequestResponseDataCommand()
-> NotifyCommonPacketFormat()
-> NotifyMessageRouter()
-> NotifyClass()
-> ForwardClose() / GetConnectionData() / SearchConnectionData()
-> GetUintFromMessage()
```

## Relevant code

### 1. TCP receive buffer is stack-backed

`HandleDataOnTcpSocket()` receives the packet into a local stack buffer:

```c
EipStatus HandleDataOnTcpSocket(int socket) {
...
CipOctet incoming_message[PC_OPENER_ETHERNET_BUFFER_SIZE] = { 0 };
...
EipStatus need_to_send = HandleReceivedExplictTcpData(socket,
incoming_message,
data_size,
&remaining_bytes,
&sender_address,
&outgoing_message);
```

This matters because the out-of-bounds read is observed beyond this `incoming_message[...]` buffer.

### 2. `SendRRData` reaches the unconnected Message Router path

The TCP explicit message handler forwards registered-session `SendRRData` traffic into CPF parsing:

```c
EipStatus HandleReceivedSendRequestResponseDataCommand(
const EncapsulationData *const receive_data,
const struct sockaddr *const originator_address,
ENIPMessage *const outgoing_message) {

if(receive_data->data_length >= 6) {
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 = NotifyCommonPacketFormat(receive_data,
originator_address,
outgoing_message);
}
}
}
```

And `NotifyCommonPacketFormat()` forwards unconnected data items into the Message Router:

```c
if(g_common_packet_format_data_item.address_item.type_id ==
kCipItemIdNullAddress) {
if(g_common_packet_format_data_item.data_item.type_id ==
kCipItemIdUnconnectedDataItem) {
return_value = NotifyMessageRouter(
g_common_packet_format_data_item.data_item.data,
g_common_packet_format_data_item.data_item.length,
&message_router_response,
originator_address,
received_data->session_handle);
}
}
```

### 3. Message Router allows path consumption to reduce request body to zero

`CreateMessageRouterRequestStructure()` parses the service code and padded EPath, then sets the request-body pointer and length to the bytes remaining **after** the path:

```c
CipError CreateMessageRouterRequestStructure(const EipUint8 *data,
EipInt16 data_length,
CipMessageRouterRequest *message_router_request)
{
message_router_request->service = *data;
data++;
data_length--;

size_t number_of_decoded_bytes;
const EipStatus path_result =
DecodePaddedEPath(&(message_router_request->request_path),
&data,
&number_of_decoded_bytes);

if(path_result != kEipStatusOk) {
return kCipErrorPathSegmentError;
}

if(number_of_decoded_bytes > data_length) {
return kCipErrorPathSizeInvalid;
} else {
message_router_request->data = data;
message_router_request->request_data_size =
data_length - number_of_decoded_bytes;
return kCipErrorSuccess;
}
}
```

In the PoC, the padded EPath is crafted so that:

- the path parses successfully,
- `message_router_request->data` ends up at the end of the CIP payload,
- `message_router_request->request_data_size == 0`.

That state is valid for some services, but **not** for the three Connection Manager services below.

### 4. `DecodePaddedEPath()` accepts and skips padded Member ID segments

The padded EPath works because `DecodePaddedEPath()` accepts Member ID segments and simply advances over them:

```c
case SEGMENT_TYPE_LOGICAL_SEGMENT + LOGICAL_SEGMENT_TYPE_MEMBER_ID +
LOGICAL_SEGMENT_FORMAT_EIGHT_BIT:
message_runner += 2;
break;

case SEGMENT_TYPE_LOGICAL_SEGMENT + LOGICAL_SEGMENT_TYPE_MEMBER_ID +
LOGICAL_SEGMENT_FORMAT_SIXTEEN_BIT:
message_runner += 2;
number_of_decoded_elements++;
break;
```

This means a long sequence of padded Member ID segments can consume the entire request payload while still producing a syntactically acceptable path.

### 5. The three affected handlers do not validate minimum request-body length

ForwardClose:

```c
message_router_request->data += 2; /* ignore Priority/Time_tick and Time-out_ticks */

EipUint16 connection_serial_number = GetUintFromMessage(
&message_router_request->data);
EipUint16 originator_vendor_id = GetUintFromMessage(
&message_router_request->data);
EipUint32 originator_serial_number = GetUdintFromMessage(
&message_router_request->data);
```

`ForwardClose` needs at least:

- 2 bytes: priority / timeout
- 2 bytes: connection serial number
- 2 bytes: vendor id
- 4 bytes: originator serial number

## So the minimum required body size here is **10 bytes**.

GetConnectionData:

```c
EipUint16 Connection_number =
GetUintFromMessage(&message_router_request->data);
```

## This handler needs at least **2 bytes**.

SearchConnectionData:

```c
EipUint16 Connection_serial_number = GetUintFromMessage(
&message_router_request->data);
EipUint16 Originator_vendor_id = GetUintFromMessage(
&message_router_request->data);
EipUint32 Originator_serial_number = GetUdintFromMessage(
&message_router_request->data);
```

This handler needs at least **8 bytes**.

### 6. The final sink is an unchecked read helper

All three handlers eventually reach the same unchecked primitive:

```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;
}
```

Once `message_router_request->data` points at or beyond the end of the receive buffer, this becomes an out-of-bounds read immediately.

## ASan evidence

I validated this with three PoC modes that only differ in the service code.

### Mode 1: `forwardclose` (`0x4E`)

Server-side ASan shows:

```bash
#0 GetUintFromMessage ... endianconv.c:75
#1 ForwardClose ... cipconnectionmanager.c:687
#2 NotifyClass ... cipcommon.c:127
#3 NotifyMessageRouter ... cipmessagerouter.c:217
#4 NotifyCommonPacketFormat ... cpf.c:60
#5 HandleReceivedSendRequestResponseDataCommand ... encap.c:558
#6 HandleReceivedExplictTcpData ... encap.c:186
#7 HandleDataOnTcpSocket ... generic_networkhandler.c:864
```

ASan reports the read beyond `incoming_message` in the `HandleDataOnTcpSocket()` frame.
The reported access offset is **562**, which matches the source because `ForwardClose()` first does `message_router_request->data += 2` before the first `GetUintFromMessage()`.

### Mode 2: `getconn` (`0x56`)

Server-side ASan shows:

```bash
#0 GetUintFromMessage ... endianconv.c:75
#1 GetConnectionData ... cipconnectionmanager.c:772
#2 NotifyClass ... cipcommon.c:127
#3 NotifyMessageRouter ... cipmessagerouter.c:217
#4 NotifyCommonPacketFormat ... cpf.c:60
#5 HandleReceivedSendRequestResponseDataCommand ... encap.c:558
#6 HandleReceivedExplictTcpData ... encap.c:186
#7 HandleDataOnTcpSocket ... generic_networkhandler.c:864
```

The reported read is at offset **560**, i.e. directly at the end of `incoming_message`, which matches the fact that `GetConnectionData()` performs no pre-skip before its first `GetUintFromMessage()`.

### Mode 3: `searchconn` (`0x57`)

Server-side ASan shows:

```bash
#0 GetUintFromMessage ... endianconv.c:75
#1 SearchConnectionData ... cipconnectionmanager.c:820
#2 NotifyClass ... cipcommon.c:127
#3 NotifyMessageRouter ... cipmessagerouter.c:217
#4 NotifyCommonPacketFormat ... cpf.c:60
#5 HandleReceivedSendRequestResponseDataCommand ... encap.c:558
#6 HandleReceivedExplictTcpData ... encap.c:186
#7 HandleDataOnTcpSocket ... generic_networkhandler.c:864
```

Again, the read occurs at offset **560**, consistent with the handler immediately reading the first field from an empty request body.

## Reproduction

### POC

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

The client PoC uses three modes that differ only in the service code.
All modes:

**1.** create a TCP connection,
**2.** perform `RegisterSession`,
**3.** send `SendRRData`,
**4.** route to `class 0x06 / instance 1` (Connection Manager),
**5.** use a long padded EPath to consume the full CIP payload,
**6.** intentionally omit the real request body.

Executions:

```bash
python3 poc.py forwardclose 192.168.153.128
python3 poc.py getconn 192.168.153.128
python3 poc.py searchconn 192.168.153.128
```

Observed client-side behavior:

```bash
target=forwardclose service=0x4e cip_len=472 packet_len=512
session=1
response=timeout

target=getconn service=0x56 cip_len=472 packet_len=512
session=1
response=timeout

target=searchconn service=0x57 cip_len=472 packet_len=512
session=1
response=timeout
```

The timeout is expected because the server aborts before sending a response.

## Root cause

This is **not** primarily an attribute-decoder issue and **not** just a generic `DecodePaddedEPath()` bug.

The actual root cause is:

- `DecodePaddedEPath()` accepts padded Member ID segments and advances over them.
- `CreateMessageRouterRequestStructure()` sets `message_router_request->data` and `request_data_size` to whatever remains after the path.
- The padded path can legally consume the entire payload, leaving `request_data_size == 0`.
- `ForwardClose`, `GetConnectionData`, and `SearchConnectionData` do **not** validate that their required fixed-size request body is present.
- They then blindly call unchecked read helpers, producing an out-of-bounds read past the stack receive buffer.

## Why this should not be split into three separate issues

Although three handlers are affected, they share:

- the same network entry path,
- the same padded EPath technique,
- the same `request_data_size == 0` condition,
- the same unchecked read primitive,
- the same stack receive buffer boundary.

The only difference is which Connection Manager service code is used and which handler performs the first invalid read.

This is therefore better treated as **one vulnerability family affecting multiple Connection Manager services**, not as three unrelated bugs.

## Suggested fix

A minimal fix is to validate `message_router_request->request_data_size` at the beginning of each affected handler before any pointer advance or read.

### Example fix direction

```c
#define FORWARD_CLOSE_MIN_REQUEST_SIZE 10U
#define GET_CONNECTION_DATA_MIN_REQUEST_SIZE 2U
#define SEARCH_CONNECTION_DATA_MIN_REQUEST_SIZE 8U
```

ForwardClose:

```c
if(message_router_request->request_data_size < FORWARD_CLOSE_MIN_REQUEST_SIZE) {
message_router_response->reply_service =
(0x80 | message_router_request->service);
message_router_response->general_status = kCipErrorNotEnoughData;
message_router_response->size_of_additional_status = 0;
return kEipStatusOkSend;
}
```

GetConnectionData:

```
if(message_router_request->request_data_size < GET_CONNECTION_DATA_MIN_REQUEST_SIZE) {
message_router_response->reply_service =
(0x80 | message_router_request->service);
message_router_response->general_status = kCipErrorNotEnoughData;
message_router_response->size_of_additional_status = 0;
return kEipStatusOkSend;
}
```

SearchConnectionData:

```
if(message_router_request->request_data_size < SEARCH_CONNECTION_DATA_MIN_REQUEST_SIZE) {
message_router_response->reply_service =
(0x80 | message_router_request->service);
message_router_response->general_status = kCipErrorNotEnoughData;
message_router_response->size_of_additional_status = 0;
return kEipStatusOkSend;
}
```

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.