Valid GetAttributeList request causes response-side `buffer overflow` via per-entry size underestimation

[CarnegieMe]

Summary

I found a remotely reachable server-side memory corruption issue in the current master branch of OpENer in the unconnected explicit-message SendRRData path.

The root cause is in GetAttributeList(): the response-space estimation for each returned attribute entry underestimates the actual encoded size by 4 bytes per attribute item. Specifically, the estimation accounts for the attribute value length, but it does not account for the fixed per-entry metadata that is always written afterward:

• Attribute ID (2 bytes)
• status (1 byte)
• reserved (1 byte)

With a valid GetAttributeList request against Identity instance 1, repeatedly requesting attribute 1, the underestimated inner Message Router response first corrupts outer response metadata during response assembly. When the request also carries two valid optional sockaddr info items (0x8000 and 0x8001), AssembleLinearMessage() continues encoding those legal CPF items and eventually crashes in AddIntToMessage() using the already-corrupted current_message_position.

---

Version

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

---

Impact

At minimum this is a remotely reachable denial of service. Because the bug corrupts response-assembly metadata before the final crash becomes visible, the underlying behavior is more severe than a simple reject/fail condition.

---

Affected area

• server
• unconnected explicit messaging
• SendRRData
• response assembly / CPF re-encoding

---

Reproduction

POC

poc.zip

Build commands

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 commands

Server:

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

Client PoC:

python3 poc.py 192.168.153.128 80 1 1

Parameter meaning:

• 192.168.153.128: server IP
• 80: GetAttributeList attribute count
• 1: repeatedly request Identity attribute 1
• 1: include both valid optional sockaddr info items

The client PoC used in this reproduction constructs a standard RegisterSession followed by a standard SendRRData request with a self-consistent CPF layout and two legal optional sockaddr items.

Observed behavior

Client:

host=192.168.153.128 count=80 attribute=1 both_items=1 cip_len=168 packet_len=248
session=1
response=timeout

Server log before crash:

Sockaddr type id: 8000
Sockaddr type id: 8001
notify: calling GetAttributeList service

ASan crash:

AddressSanitizer:DEADLYSIGNAL
=================================================================
==108503==ERROR: AddressSanitizer: SEGV on unknown address 0x20007fffa03c
==108503==The signal is caused by a READ memory access.
    #0  AddIntToMessage .../source/src/enet_encap/endianconv.c:136:49
    #1  EncodeSockaddrInfoItemTypeId .../source/src/enet_encap/cpf.c:595:3
    #2  AssembleLinearMessage .../source/src/enet_encap/cpf.c:696:9
    #3  NotifyCommonPacketFormat .../source/src/enet_encap/cpf.c:70:30
    #4  HandleReceivedSendRequestResponseDataCommand .../source/src/enet_encap/encap.c:558:22
    #5  HandleReceivedExplictTcpData .../source/src/enet_encap/encap.c:186:26
    #6  HandleDataOnTcpSocket .../source/src/ports/generic_networkhandler.c:864:30

The client-side timeout is expected here: the server crashes during response assembly before a complete reply is sent.

Reachable call chain

main()
  -> executeEventLoop()
    -> NetworkHandlerProcessCyclic()
      -> HandleDataOnTcpSocket()
        -> HandleReceivedExplictTcpData()
          -> HandleReceivedSendRequestResponseDataCommand()
            -> NotifyCommonPacketFormat()
              -> NotifyMessageRouter()
                -> Identity::GetAttributeList()
                  -> AssembleLinearMessage()
                    -> EncodeSockaddrInfoItemTypeId()
                      -> AddIntToMessage()

Root cause analysis

1. GetAttributeList() underestimates the response size for each returned attribute

Current code in GetAttributeList() only estimates the attribute value size for an existing attribute, or two CipSint values for a missing attribute:

const int_fast64_t needed_message_space = NULL != attribute
    ? (int_fast64_t) GetCipDataTypeLength(attribute->type,
                                          attribute->data)
    : (int_fast64_t) (2 * sizeof(CipSint));

But each response entry actually writes more than that.

Relevant code path:

AddIntToMessage(attribute_number, &message_router_response->message);  // Attribute-ID

if(NULL != attribute) {
  ...
  AddSintToMessage(kCipErrorSuccess, &message_router_response->message); // status
  AddSintToMessage(0, &message_router_response->message);                // reserved
  attribute->encode(attribute->data, &message_router_response->message); // value
} else {
  AddSintToMessage(kCipErrorAttributeNotSupported,
                   &message_router_response->message);
  AddSintToMessage(0, &message_router_response->message);
}

So for every successfully returned attribute entry, the real encoded size is:

• Attribute ID: 2 bytes
• status: 1 byte
• reserved: 1 byte
• attribute value: GetCipDataTypeLength(...)

That is:

real_size_per_successful_entry = 4 + value_length
estimated_size_per_successful_entry = value_length
underestimation_per_successful_entry = 4

For the PoC request against Identity instance 1, attribute 1 is a valid UINT attribute, so the server repeatedly takes the successful path. The script requests it 80 times, so the total underestimation accumulates linearly.

Relevant source excerpt from master:

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

  const int_fast64_t needed_message_space = NULL != attribute
      ? (int_fast64_t) GetCipDataTypeLength(attribute->type,
                                            attribute->data)
      : (int_fast64_t) (2 * sizeof(CipSint));

  const int_fast64_t remaining_message_space =
    (int_fast64_t) PC_OPENER_ETHERNET_BUFFER_SIZE -
    (int_fast64_t) message_router_response->message.used_message_length -
    33LL;

  if (needed_message_space > remaining_message_space) {
    ...
    return kEipStatusOkSend;
  }

  AddIntToMessage(attribute_number, &message_router_response->message);

  if(NULL != attribute) {
    ...
    AddSintToMessage(kCipErrorSuccess, &message_router_response->message);
    AddSintToMessage(0, &message_router_response->message);
    attribute->encode(attribute->data, &message_router_response->message);
  } else {
    AddSintToMessage(kCipErrorAttributeNotSupported,
                     &message_router_response->message);
    AddSintToMessage(0, &message_router_response->message);
    message_router_response->general_status = kCipErrorAttributeListError;
  }
}

2. The PoC uses a self-consistent CPF and legal optional sockaddr items

The PoC constructs the SendRRData payload as:

• Interface Handle = 0
• Timeout = 0
• CPF item count = 4
• Null Address Item
• Unconnected Data Item (0x00B2)
• sockaddr info item 0x8000
• sockaddr info item 0x8001

This matches the server-side CPF parser logic. The relevant parser accepts up to two optional address info items after the first two required items and parses 0x8000 / 0x8001 as sockaddr items:

CipUint address_item_count = (CipUint)(common_packet_format_data->item_count - 2U);

for(size_t j = 0; j < (address_item_count > 2 ? 2 : address_item_count); j++) {
  common_packet_format_data->address_info_item[j].type_id =
    GetIntFromMessage(&data);

  if((common_packet_format_data->address_info_item[j].type_id ==
      kCipItemIdSocketAddressInfoOriginatorToTarget)
     || (common_packet_format_data->address_info_item[j].type_id ==
         kCipItemIdSocketAddressInfoTargetToOriginator)) {
    common_packet_format_data->address_info_item[j].length =
      GetIntFromMessage(&data);
    common_packet_format_data->address_info_item[j].sin_family =
      GetIntFromMessage(&data);
    common_packet_format_data->address_info_item[j].sin_port =
      GetIntFromMessage(&data);
    common_packet_format_data->address_info_item[j].sin_addr =
      GetUdintFromMessage(&data);
    ...
  }
}

In other words, this PoC does not rely on malformed CPF lengths or inconsistent item layout. The two optional sockaddr items are parsed as intended and the server log confirms that by printing:

Sockaddr type id: 8000
Sockaddr type id: 8001

3. The corrupted response state becomes visible when the outer CPF response is assembled

After the inner Message Router response has grown based on the underestimated per-entry size, AssembleLinearMessage() linearizes the full outer response.

Relevant response-assembly code:

if(message_router_response) {
  AddDintToMessage(0, outgoing_message);
  AddIntToMessage(0, outgoing_message);
}

EncodeItemCount(common_packet_format_data_item, outgoing_message);
...
EncodeReplyService(message_router_response, outgoing_message);
EncodeReservedFieldOfLengthByte(message_router_response, outgoing_message);
EncodeGeneralStatus(message_router_response, outgoing_message);
EncodeExtendedStatus(message_router_response, outgoing_message);
EncodeMessageRouterResponseData(message_router_response, outgoing_message);

EncodeMessageRouterResponseData() performs a raw memcpy() into the outer ENIPMessage without a remaining-capacity check:

void EncodeMessageRouterResponseData(
  const CipMessageRouterResponse *const message_router_response,
  ENIPMessage *const outgoing_message) {
  memcpy(outgoing_message->current_message_position,
         message_router_response->message.message_buffer,
         message_router_response->message.used_message_length);

  outgoing_message->current_message_position +=
    message_router_response->message.used_message_length;
  outgoing_message->used_message_length +=
    message_router_response->message.used_message_length;
}

After that, AssembleLinearMessage() continues processing the optional sockaddr response items:

for(int type = kCipItemIdSocketAddressInfoOriginatorToTarget;
    type <= kCipItemIdSocketAddressInfoTargetToOriginator; type++) {
  for(int j = 0; j < 2; j++) {
    if(common_packet_format_data_item->address_info_item[j].type_id == type) {
      EncodeSockaddrInfoItemTypeId(j,
                                   common_packet_format_data_item,
                                   outgoing_message);

      EncodeSockaddrInfoLength(j,
                               common_packet_format_data_item,
                               outgoing_message);

      EncapsulateIpAddress(
        common_packet_format_data_item->address_info_item[j].sin_port,
        common_packet_format_data_item->address_info_item[j].sin_addr,
        outgoing_message);

      FillNextNMessageOctetsWithValueAndMoveToNextPosition(0,
                                                           8,
                                                           outgoing_message);
      break;
    }
  }
}

At this point, the response metadata has already been damaged by the earlier underestimation, and the legal sockaddr item encoding becomes the first place where the broken current_message_position is dereferenced.

The final visible crash is therefore:

void AddIntToMessage(const EipUint16 data,
                     ENIPMessage *const outgoing_message) {
  outgoing_message->current_message_position[0] = (unsigned char) data;
  outgoing_message->current_message_position[1] = (unsigned char) (data >> 8);
  outgoing_message->current_message_position += 2;
  outgoing_message->used_message_length += 2;
}

The crash site is here, but the first bug is earlier: the per-entry response-space underestimation in GetAttributeList().

Security impact

A remote unauthenticated attacker that can reach the OpENer TCP explicit-message service can crash the server with a valid RegisterSession + SendRRData sequence.

Impact at minimum:

• remote denial of service

Security-relevant behavior beyond DoS:

• response-assembly metadata is corrupted before the final crash becomes visible
• the crash occurs only after earlier message state has already been damaged

Key source locations

• source/src/cip/cipcommon.c — GetAttributeList() response-space estimate and actual per-entry writes
• source/src/enet_encap/cpf.c — outer response assembly and optional sockaddr response encoding
• source/src/enet_encap/endianconv.c — final crash in AddIntToMessage()
• source/src/enet_encap/encap.c — SendRRData entry point
• source/src/ports/generic_networkhandler.c — TCP receive path
• source/src/ports/POSIX/main.c — POSIX server entry point

Fix direction

1. Fix the size estimation in GetAttributeList()

The estimation should include the fixed 4-byte per-entry overhead.

A minimal correction would be conceptually:

const int_fast64_t fixed_entry_overhead =
  sizeof(CipUint) +      /* Attribute ID */
  sizeof(CipSint) +      /* status */
  sizeof(CipSint);       /* reserved */

const int_fast64_t value_length =
  NULL != attribute
    ? (int_fast64_t) GetCipDataTypeLength(attribute->type, attribute->data)
    : 0;

const int_fast64_t needed_message_space =
  fixed_entry_overhead + value_length;

This should be applied consistently so that both success and error entries are budgeted according to what is actually written.

2. Add a hard remaining-capacity check in outer response assembly

EncodeMessageRouterResponseData() should not blindly memcpy() into outgoing_message without verifying the remaining capacity.

A minimal defensive check would be conceptually:

size_t remaining =
  PC_OPENER_ETHERNET_BUFFER_SIZE - outgoing_message->used_message_length;

if(message_router_response->message.used_message_length > remaining) {
  return; /* or propagate an explicit error */
}

A proper fix should propagate a failure status rather than continuing with a partially corrupted response.

3. Add capacity checks before encoding optional sockaddr items

The sockaddr response encoding path in AssembleLinearMessage() should also validate the remaining space before writing:

• type id
• length
• IP/port payload
• 8-byte zero padding

This should not rely on earlier response-size predictions always being correct.