Possible out-of-bounds read (heap over-read + disclosure to JS) in `DcpIdentifyWorker::BuildHostFromFrame`

[OvOhao]

Possible out-of-bounds read (heap over-read + disclosure to JS) in DcpIdentifyWorker::BuildHostFromFrame

I found a possible OOB read in DcpIdentifyWorker::BuildHostFromFrame. When a PROFINET DCP
Identify-Response frame is captured off the network, the per-block field usDcpBlockLength
(a 16-bit big-endian length taken directly from the untrusted packet) is used to read the
block payload without ever being bounded against the number of bytes actually captured
(len) or even against the header-declared usDcpDataLength. A malicious/malformed device on
the L2 segment can declare a block length of up to 65535, causing the addon to read tens of
kilobytes past the end of the ~1500-byte pcap capture buffer. For the string-valued blocks
(NameOfStation / Alias / Vendor) the over-read bytes are copied into a std::string and
handed straight back to JavaScript, i.e. it is a heap-disclosure primitive.

File: src/dcp.cc

Function: DcpIdentifyWorker::BuildHostFromFrame (via the PROCESS_STRING_PROP macro)

#define PROCESS_STRING_PROP(SubOption, Key) \
        if (pDcpBlock->bSubOption == SubOption && usDcpBlockLength > 2) { \
          std::string stringProp(""); \
          stringProp.append((char*)(pBlockData + 2), usDcpBlockLength - 2); \
          host.Set(Key, stringProp.c_str()); \
        }
...
  DCP_RESPONSE_BLOCK_HEADER* pDcpBlock = (DCP_RESPONSE_BLOCK_HEADER*)(pDcpHeader + 1);
  u_short usDcpBlockLength = ntohs(pDcpBlock->usDcpBlockLength);

  for (size_t processed = 0; processed < (size_t)ntohs(pDcpHeader->usDcpDataLength);
       processed += (sizeof(DCP_RESPONSE_BLOCK_HEADER) + usDcpBlockLength + (usDcpBlockLength % 2))) {
    pDcpBlock = (DCP_RESPONSE_BLOCK_HEADER*)((u_char*)(pDcpHeader + 1) + processed);
    usDcpBlockLength = ntohs(pDcpBlock->usDcpBlockLength);   // attacker-controlled, unbounded
    u_char* pBlockData = (u_char*)(pDcpBlock + 1);

    switch (pDcpBlock->bOption) {
    ...
    case OPTION_DEVPROP:
      PROCESS_STRING_PROP(DEVPROP_NAMEOFSTATION, "NameOfStation"); // reads usDcpBlockLength-2 bytes
      PROCESS_STRING_PROP(DEVPROP_ALIAS, "Alias");
      PROCESS_STRING_PROP(DEVPROP_DEVICEVENDOR, "Vendor");

Path, line by line:

1. OnProgress (caller) only validates the aggregate DCP data region:
   len >= ethHeaderSize + sizeof(DCP_RESPONSE_HEADER) + usDcpDataLength. It never validates
   the individual block length fields inside that region.
2. In BuildHostFromFrame the loop iterates while processed < usDcpDataLength. On each
   iteration it reads usDcpBlockLength = ntohs(pDcpBlock->usDcpBlockLength) straight from the
   packet. Nothing constrains this value to usDcpDataLength - processed or to len.
   (The constants DCP_MAX_NAME_LENGTH / DCP_MAX_LABEL_LENGTH are defined in dcp_pdu.h but
   never used here.)
3. For a Device-Properties block whose sub-option is NameOfStation/Alias/DeviceVendor,
   PROCESS_STRING_PROP executes stringProp.append((char*)(pBlockData + 2), usDcpBlockLength - 2).
   With usDcpBlockLength == 0xFFFF this copies 65533 bytes starting at pBlockData + 2.
4. The capture buffer returned by pcap_next is at most snaplen (1500) bytes, so the
   std::string::append reads far past the end of the heap allocation — an out-of-bounds read.
5. The result is stored via host.Set(Key, stringProp.c_str()) and the host object is passed to
   JS (Callback().Call({ host }) and resolved in the hosts array), disclosing the
   over-read heap contents to the caller.

The DEVPROP_DEVICEOPTIONS branch has the same root cause: it loops
for (i < (usDcpBlockLength - 2)/2) reading pBlockData + 2 + i*2 with the same unbounded
usDcpBlockLength, disclosing the over-read bytes as Option/SubOption numbers.

A minimal trigger is a DCP block that declares a small usDcpDataLength (so the frame length
check passes) but an enormous usDcpBlockLength, e.g. header data-length = 8 with a single
NameOfStation block claiming block-length 0xFFFF.

JS trigger (attacker-side, over the wire):

// Victim runs a normal identify scan:
const dcp = require('node-profinetdcp');
await dcp.identify(iface);   // sends the multicast Identify request
// An attacker on the same L2 segment observes the request's Xid and replies with a crafted
// DCP Identify-Response: ServiceType=success, DCPDataLength=0x0008, one DevProp/NameOfStation
// block with BlockLength=0xFFFF and only a couple of payload bytes. The addon then over-reads
// ~64KB of heap and returns it as `host.NameOfStation`.

Suggested fix: before dereferencing/reading each block, clamp against the remaining validated
region. Compute remaining = usDcpDataLength - processed, reject the block if
sizeof(DCP_RESPONSE_BLOCK_HEADER) + usDcpBlockLength > remaining (and also verify
processed + sizeof(DCP_RESPONSE_BLOCK_HEADER) <= usDcpDataLength before reading the block
header), and bound every pBlockData access by usDcpBlockLength and by the captured len.
Break out of the loop on any inconsistency instead of trusting the wire-supplied length.