`SetAttributeList` performs an out-of-bounds read on a non-settable `kCipShortString` attribute in the default build via real TCP `SendRRData`

## Summary

I found a remotely reachable server-side `out-of-bounds read` in the current `master` branch of OpENer.

The bug is triggered in the real TCP explicit-message path (`44818` / `RegisterSession` / `SendRRData`) when `SetAttributeList` targets a **real, default-registered, non-settable** `kCipShortString` attribute: **Identity class, instance 1, attribute 7 (`product_name`)**.

The root cause is that `SetAttributeList()` still tries to **skip** the incoming attribute value even when the attribute is not settable. For variable-length types, it calls `GetCipDataTypeLength(attribute->type, message_router_request->data)` on attacker-controlled request data without any remaining-length validation. For `kCipShortString`, this immediately performs an unbounded 1-byte read of the string length header through `GetSintFromMessage()`.

By crafting a long padded EPath so that `message_router_request->data` is positioned at the end of the TCP stack receive buffer, the length-header read crosses the right boundary of `incoming_message[512]` and ASan reports a `stack-buffer-overflow` (OOB read).

At minimum, this is a **remote unauthenticated denial of service** in the default build.

---

## Version

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

## Affected path

Real request handling path:

```text
main()
-> executeEventLoop()
-> NetworkHandlerProcessCyclic()
-> HandleDataOnTcpSocket()
-> HandleReceivedExplictTcpData()
-> HandleReceivedSendRequestResponseDataCommand()
-> NotifyCommonPacketFormat()
-> NotifyMessageRouter()
-> NotifyClass()
-> SetAttributeList()
-> GetCipDataTypeLength()
-> GetSintFromMessage()
```

## Root cause

### 1) `SetAttributeList()` reads attribute IDs, then enters the "attribute exists but is not settable" branch

```c
CipUint attribute_count_request = GetUintFromMessage(
&message_router_request->data);

...

for(size_t j = 0; j < attribute_count_request; j++) {
attribute_number = GetUintFromMessage(&message_router_request->data);
attribute = GetCipAttribute(instance, attribute_number);

...

if(NULL != attribute) {

uint8_t set_bit_mask =
(instance->cip_class->set_bit_mask[CalculateIndex(attribute_number)]);
if( 0 != ( set_bit_mask & ( 1 << (attribute_number % 8) ) ) ) {
AddSintToMessage(kCipErrorSuccess, &message_router_response->message);
AddSintToMessage(0, &message_router_response->message);
attribute->decode(attribute->data,
message_router_request,
message_router_response);
} else {
AddSintToMessage(kCipErrorAttributeNotSetable,
&message_router_response->message);
AddSintToMessage(0, &message_router_response->message);

// move request message pointer
size_t attribute_data_length = GetCipDataTypeLength(attribute->type,
message_router_request->data);
if(0 != attribute_data_length) {
message_router_request->data += attribute_data_length;
message_router_response->general_status =
kCipErrorAttributeListError;
} else {
message_router_response->general_status = kCipErrorPartialTransfer;
return kEipStatusOkSend;
}
}
}
}
```

The bug is here: **for a non-settable attribute, the code still tries to parse/skip the attacker-supplied value from request data**, but does not validate whether enough bytes remain.

### 2) `GetCipDataTypeLength()` performs a raw length-header read for `kCipShortString`

```c
case kCipShortString:
if(NULL != data){
length = GetSintFromMessage(&data) + 1; // string length + 1 byte length indicator
}
break;
```

This function receives only a pointer, not a remaining-length argument.

### 3) `GetSintFromMessage()` is an unchecked 1-byte read

```c
CipSint GetSintFromMessage(const EipUint8 **const buffer) {
const unsigned char *const buffer_address = (unsigned char *) *buffer;
EipUint8 data = buffer_address[0];
*buffer += 1;
return data;
}
```

So once `message_router_request->data` is at the end of the TCP receive buffer, this immediately becomes an OOB read.

### 4) `CreateMessageRouterRequestStructure()` stores the post-path pointer as request data

```c
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);

...

message_router_request->data = data;
message_router_request->request_data_size = data_length -
number_of_decoded_bytes;
```

A long padded EPath can therefore push `message_router_request->data` very close to the end of the TCP input buffer.

### 5) The TCP explicit-message receive buffer is stack-backed and 512 bytes by default

```c
CipOctet incoming_message[PC_OPENER_ETHERNET_BUFFER_SIZE] = { 0 };
```

Default build configuration:

```
set( OPENER_ETHERNET_BUFFER_SIZE "512" CACHE STRING "Number of bytes used for the Ethernet message buffer")
add_definitions(-DPC_OPENER_ETHERNET_BUFFER_SIZE=${OPENER_ETHERNET_BUFFER_SIZE} )
```

## Trigger condition

A crafted `SendRRData` request uses a long padded EPath to place `message_router_request->data` near the end of `incoming_message[512]`.

Then `SetAttributeList()` consumes:

- 2 bytes: `attribute_count = 1`
- 2 bytes: `attribute_id = 7`

At that point, the not-settable branch tries to compute the length of the attacker-supplied `kCipShortString` value by reading its 1-byte length header:

```c
GetCipDataTypeLength(kCipShortString, message_router_request->data)
-> GetSintFromMessage(&data)
```

That length-header read crosses the right boundary of `incoming_message[512]`.

## Reproduction

### POC

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

### Build

```bash
cmake -S source -B build \
-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 --target OpENer -j"$(nproc)"
```

### Run server

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

### Baseline packet

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

Expected baseline response:

```bash
reply_service=0x84 general_status=0x0a additional_status=[] payload=010007000e00
```

This shows that the default build really reaches `Identity` / `SetAttributeList` / attribute 7, and that attribute 7 is present but not settable.

### Crash packet

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

Typical client-side result:

```bash
response=timeout
```

Server-side ASan result:

```bash
AddressSanitizer: stack-buffer-overflow
#0 GetSintFromMessage
#1 GetCipDataTypeLength
#2 SetAttributeList
...
```

## ASan Outcome

Observed crash:

```bash
==84907==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffd7cfea830
READ of size 1 at 0x7ffd7cfea830 thread T0
#0 GetSintFromMessage ... endianconv.c:33
#1 GetCipDataTypeLength ... ciptypes.c:70
#2 SetAttributeList ... cipcommon.c:1302
#3 NotifyClass ... cipcommon.c:127
#4 NotifyMessageRouter ... cipmessagerouter.c:217
#5 NotifyCommonPacketFormat ... cpf.c:60
#6 HandleReceivedSendRequestResponseDataCommand ... encap.c:558
#7 HandleReceivedExplictTcpData ... encap.c:186
#8 HandleDataOnTcpSocket ... generic_networkhandler.c:864
```

ASan identifies the overflown object as the TCP receive stack buffer:

```bash
Address 0x7ffd7cfea830 is located in stack of thread T0 at offset 560 in frame
#0 HandleDataOnTcpSocket ... generic_networkhandler.c:720

This frame has 6 object(s):
[48, 560) 'incoming_message' (line 732) <== Memory access at offset 560 overflows this variable
```

This is exactly consistent with a 1-byte OOB read of the short-string length header beyond `incoming_message[512]`.

## Security impact

This issue is reachable in the **default build** through the real TCP explicit-message path.

The currently demonstrated impact is a reliable **remote unauthenticated denial of service** through an `out-of-bounds read` on `stack-backed` request data.

Although the demonstrated primitive is a read, the code also treats the adjacent out-of-bounds byte as a `kCipShortString` length header and uses it to advance the request pointer, so the bug should be treated as a real `memory-safety` issue rather than a benign parse failure.

## Why this is distinct from other recently reported issues

This issue is **not** the same root cause as:

- CPF payload lifetime / stack-use-after-return issues
- `GetAttributeList()` response assembly overflows
- connected-path underflow into `DecodePaddedEPath`
- `SetAttributeSingle()` cases where an empty value is passed to an attribute decoder

Here, the vulnerability is specifically:

- in `SetAttributeList()`
- in the **not-settable attribute** branch
- while trying to skip a **variable-length** request value
- through `GetCipDataTypeLength(kCipShortString, attacker_controlled_pointer)`
- which performs an unchecked length-header read

##Suggested fix direction

The core problem is that variable-length parsing is done from a raw pointer without a remaining-length argument.

### 1) Track and validate remaining request bytes inside `SetAttributeList()`

Instead of only advancing `message_router_request->data`, keep a local remaining-length variable and decrement it after every read.

Minimal direction:

```c
size_t remaining_request_data_size = message_router_request->request_data_size;

if (remaining_request_data_size < sizeof(CipUint)) {
message_router_response->general_status = kCipErrorPartialTransfer;
return kEipStatusOkSend;
}

CipUint attribute_count_request = GetUintFromMessage(&message_router_request->data);
remaining_request_data_size -= sizeof(CipUint);

for (size_t j = 0; j < attribute_count_request; j++) {
if (remaining_request_data_size < sizeof(CipUint)) {
message_router_response->general_status = kCipErrorPartialTransfer;
return kEipStatusOkSend;
}

attribute_number = GetUintFromMessage(&message_router_request->data);
remaining_request_data_size -= sizeof(CipUint);

...
}
```

## 2) Replace `GetCipDataTypeLength()` with a checked variant that takes remaining length

Example direction:

```c
EipBool8 GetCipDataTypeLengthChecked(CipDataType type,
const EipUint8 *data,
size_t remaining_bytes,
size_t *out_length) {
switch(type) {
case kCipShortString:
if (remaining_bytes < 1) {
return false;
}
*out_length = (size_t)data[0] + 1;
if (*out_length > remaining_bytes) {
return false;
}
return true;

case kCipString:
case kCipString2:
case kCipStringN:
if (remaining_bytes < 2) {
return false;
}
*out_length = (size_t)(data[0] | (data[1] << 8)) + 2;
if (*out_length > remaining_bytes) {
return false;
}
return true;

default:
...
}
}
```

### 3) In the "attribute not settable" branch, fail if the full value is not present

Instead of blindly doing:

```c
size_t attribute_data_length = GetCipDataTypeLength(attribute->type,
message_router_request->data);
message_router_request->data += attribute_data_length;
```

do:

```c
size_t attribute_data_length = 0;
if (!GetCipDataTypeLengthChecked(attribute->type,
message_router_request->data,
remaining_request_data_size,
&attribute_data_length)) {
message_router_response->general_status = kCipErrorPartialTransfer;
return kEipStatusOkSend;
}

message_router_request->data += attribute_data_length;
remaining_request_data_size -= attribute_data_length;
message_router_response->general_status = kCipErrorAttributeListError;
```

### 4) Optional hardening

A smaller short-term mitigation would be to special-case the not-settable path and reject truncated or malformed values immediately, but the more robust fix is to stop parsing variable-length types anywhere in the stack without carrying a remaining-length parameter.