ForwardOpen: 32-bit logical class/instance/connection-point segments desynchronize `remaining_path` and trigger a reachable stack-buffer-overflow read in `ParseConnectionPath()`

[CarnegieMe]

Summary

ForwardOpen on current master mis-accounts the remaining connection-path length when small numeric logical class / instance / connection-point segments are encoded as 32-bit logical segments.

The parser advances message according to the actual logical segment encoding width in CipEpathGetLogicalValue(), but ParseConnectionPath() decrements remaining_path according to the decoded value size heuristic (or even a fixed -1 for class), not according to the segment's actual on-wire width.

As a result, message can be moved past the end of the received TCP packet while remaining_path still stays positive. The later configuration-data loop then continues parsing and calls GetPathSegmentType(message) on an out-of-bounds pointer, producing a reproducible stack-buffer-overflow read on the server's TCP receive buffer.

This is reachable through the real server path:

TCP:44818 -> RegisterSession -> SendRRData -> CPF -> Message Router -> ForwardOpen -> ParseConnectionPath

Version

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

Security impact

This issue is a reachable, unauthenticated network-triggerable stack out-of-bounds read in the ForwardOpen connection-path parser on the default sample server.

From the current evidence, the reliable impact is:

• remote crash / denial of service
• parser reads beyond the end of the TCP receive buffer while interpreting attacker-controlled connection-path bytes

The current evidence supports a real memory-safety bug and remote DoS. It does not require source modification on the server side.

Affected code path

Real server-side entry and parsing chain:

main()
  -> executeEventLoop()
  -> NetworkHandlerProcessCyclic()
  -> HandleDataOnTcpSocket()
  -> HandleReceivedExplictTcpData()
  -> HandleReceivedSendRequestResponseDataCommand()
  -> NotifyCommonPacketFormat()
  -> NotifyMessageRouter()
  -> ForwardOpen()
  -> ForwardOpenRoutine()
  -> HandleNonNullNonMatchingForwardOpenRequest()
  -> ParseConnectionPath()
  -> GetPathSegmentType()

Relevant source files on current master:

• source/src/ports/POSIX/main.c
• source/src/ports/generic_networkhandler.c
• source/src/enet_encap/encap.c
• source/src/enet_encap/cpf.c
• source/src/cip/cipconnectionmanager.c
• source/src/cip/cipepath.c

Default sample-app reachability

The default POSIX sample application exposes a valid Assembly I/O path that is directly usable by ForwardOpen:

#define DEMO_APP_INPUT_ASSEMBLY_NUM                100 //0x064
#define DEMO_APP_OUTPUT_ASSEMBLY_NUM               150 //0x096
#define DEMO_APP_CONFIG_ASSEMBLY_NUM               151 //0x097

ConfigureExclusiveOwnerConnectionPoint(0, DEMO_APP_OUTPUT_ASSEMBLY_NUM,
                                       DEMO_APP_INPUT_ASSEMBLY_NUM,
                                       DEMO_APP_CONFIG_ASSEMBLY_NUM);

So the bug is reachable on the unmodified default sample app by targeting:

• class = Assembly (0x04)
• config instance = 151
• O->T connection point = 150
• T->O connection point = 100

Root cause

1) Actual logical-segment consumption is width-dependent

CipEpathGetLogicalValue() consumes bytes according to the actual logical segment format:

CipDword CipEpathGetLogicalValue(const EipUint8 **message) {
  LogicalSegmentLogicalFormat logical_format =
    GetPathLogicalSegmentLogicalFormat(*message);
  CipDword data = kLogicalSegmentLogicalFormatInvalid;
  (*message) += 1; /* Move to logical value */
  switch(logical_format) {
    case kLogicalSegmentLogicalFormatEightBit:
      data = GetByteFromMessage(message);
      break;
    case kLogicalSegmentLogicalFormatSixteenBit:
      (*message) += 1; /* Pad byte needs to be skipped */
      data = GetWordFromMessage(message);
      break;
    case kLogicalSegmentLogicalFormatThirtyTwoBit:
      (*message) += 1; /* Pad byte needs to be skipped */
      data = GetDwordFromMessage(message);
      break;
    default:
      ...
  }
  return data;
}

This means the parser pointer advances by the real encoded width:

• 8-bit logical segment -> 1 word
• 16-bit logical segment -> 2 words
• 32-bit logical segment -> 3 words

2) ParseConnectionPath() decrements remaining_path inconsistently

For the logical class segment, remaining_path is always decremented by exactly one word, regardless of actual encoding width:

if(kSegmentTypeLogicalSegment == GetPathSegmentType(message) &&
   kLogicalSegmentLogicalTypeClassId ==
   GetPathLogicalSegmentLogicalType(message) ) {

  class_id = CipEpathGetLogicalValue(&message);
  class = GetCipClass(class_id);
  ...
} else {
  ...
}
remaining_path -= 1; /* 1 16Bit word for the class part of the path */

For the logical configuration instance, the decrement is based on the decoded value, not the actual on-wire logical format:

const CipDword temp_id = CipEpathGetLogicalValue(&message);
...
instance_id = (CipInstanceNum)temp_id;
/* 1 or 2 16Bit words for the configuration instance part of the path  */
remaining_path -= (instance_id > 0xFF) ? 2 : 1; //TODO: 32 bit case missing

For the connection-point segments, the same value-based logic is used:

const CipDword temp_instance_id = CipEpathGetLogicalValue(&message);
...
instance_id = (CipInstanceNum)temp_instance_id;
...
/* 1 or 2 16Bit word for the connection point part of the path */
remaining_path -= (instance_id > 0xFF) ? 2 : 1;

3) Parser state desynchronization

If small values such as:

• class = 0x04
• config instance = 151
• connection points = 150, 100

are intentionally encoded as 32-bit logical segments, CipEpathGetLogicalValue() actually consumes 3 words each, but ParseConnectionPath() only subtracts:

• class: 1
• config instance: 1 (because 151 <= 0xFF)
• connection point: 1
• connection point: 1

So the parser consumes 12 words but only decrements remaining_path by 4 words.

This creates a desync between:

• the real parser pointer: message
• the accounting variable: remaining_path

4) The later config-data loop reads past the TCP receive buffer

After the logical path is parsed, ParseConnectionPath() continues while remaining_path > 0:

while(remaining_path > 0) { /* remaining_path_size something left in the path should be configuration data */

  SegmentType segment_type = GetPathSegmentType(message);
  switch(segment_type) {
    case kSegmentTypeDataSegment: {
      ...

At this point, message may already point past the end of the actual received packet, but remaining_path is still positive because of the accounting bug. The next GetPathSegmentType(message) dereferences an out-of-bounds byte.

The dereference itself is immediate in GetPathSegmentType():

SegmentType GetPathSegmentType(const CipOctet *const cip_path) {
  const unsigned int kSegmentTypeMask = 0xE0;
  const unsigned int segment_type = *cip_path & kSegmentTypeMask;
  ...
}

Reproduction

Build

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

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

PoC

poc.zip

python3 poc.py valid 192.168.153.128
python3 poc.py baseline 192.168.153.128
python3 poc.py crash 192.168.153.128

Expected behavior:

Valid packet:

mode=valid ... path_len=8 path_words=4 ...
reply_service=0xd4 general_status=0x00

Baseline packet:

Same large path and filler, but no crash:

mode=baseline ... path_len=430 path_words=215 cip_len=472 packet_len=512
reply_service=0xd4 general_status=0x01 additional_status=[...]

Crash packet

mode=crash ... path_len=430 path_words=215 cip_len=472 packet_len=512
response=timeout

ASan evidence

The server crashes with a real stack out-of-bounds read:

==96711==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fffb2c59050
READ of size 1 at 0x7fffb2c59050 thread T0
    #0  GetPathSegmentType .../source/src/cip/cipepath.c:22
    #1  ParseConnectionPath .../source/src/cip/cipconnectionmanager.c:1624
    #2  HandleNonNullNonMatchingForwardOpenRequest .../source/src/cip/cipconnectionmanager.c:472
    #3  ForwardOpenRoutine .../source/src/cip/cipconnectionmanager.c:662
    #4  ForwardOpen .../source/src/cip/cipconnectionmanager.c:579
    #5  NotifyClass .../source/src/cip/cipcommon.c:127
    #6  NotifyMessageRouter .../source/src/cip/cipmessagerouter.c:217
    #7  NotifyCommonPacketFormat .../source/src/enet_encap/cpf.c:60
    #8  HandleReceivedSendRequestResponseDataCommand .../source/src/enet_encap/encap.c:558
    #9  HandleReceivedExplictTcpData .../source/src/enet_encap/encap.c:186
    #10 HandleDataOnTcpSocket .../source/src/ports/generic_networkhandler.c:864

ASan also points directly to the TCP receive stack buffer:

Address ... is located in stack of thread T0 at offset 560 in frame
    #0 HandleDataOnTcpSocket .../source/src/ports/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 matches the real network receive path and shows that the fault is not a synthetic unit-test artifact.

Minimal fix direction

The bug should be fixed at the accounting layer: remaining_path must be decremented according to the actual encoded logical segment width, not according to the decoded value.

A minimal and robust approach is:

1. determine the logical segment format from the current segment header before calling CipPathGetLogicalValue();

2. convert that format into the exact number of 16-bit words consumed:

   • 8-bit -> 1 word
   • 16-bit -> 2 words
   • 32-bit -> 3 words

3. verify consumed_words <= remaining_path before consuming the segment;

4. decrement remaining_path by consumed_words for class / instance / connection-point uniformly.

For example:

static size_t GetLogicalSegmentWordCount(const EipUint8 *message) {
  switch(GetPathLogicalSegmentLogicalFormat(message)) {
    case kLogicalSegmentLogicalFormatEightBit:
      return 1;
    case kLogicalSegmentLogicalFormatSixteenBit:
      return 2;
    case kLogicalSegmentLogicalFormatThirtyTwoBit:
      return 3;
    default:
      return 0;
  }
}

Then the class / instance / connection-point handling should follow this pattern:

size_t consumed_words = GetLogicalSegmentWordCount(message);
if(0 == consumed_words || consumed_words > remaining_path) {
  *extended_error =
    kConnectionManagerExtendedStatusCodeErrorInvalidSegmentTypeInPath;
  return kCipErrorConnectionFailure;
}

value = CipEpathGetLogicalValue(&message);
remaining_path -= consumed_words;

This should replace all three current accounting patterns:

remaining_path -= 1;
remaining_path -= (instance_id > 0xFF) ? 2 : 1;
remaining_path -= (instance_id > 0xFF) ? 2 : 1;