Pre-auth heap buffer overflow in S7 Download handler — snap7 server

[entropy1337]

[CWE-191/CWE-787] Pre-auth heap buffer overflow in S7 Download handler — snap7 server

Finding ID: SNAP7-02 Severity: Critical (unauthenticated remote, potential RCE)
Component: snap7 S7 server — TS7Worker::PerformFunctionDownload()

Summary

The S7 Download handler allocates a heap buffer of an attacker-controlled size
(BlkLen, taken from the StartDownload request) and then fills it in a receive
loop whose per-block copy length is computed as
DataLen = SwapWord(ResHeader->DataLen) - 4, where DataLen is a 16-bit word.
A block whose S7 header DataLen field is < 4 underflows this subtraction to a
value near 65532. The loop performs memcpy(buffer + written, ..., DataLen) with
no written + DataLen <= BlkLen bound (it only checks written < BlkLen
before each iteration), so a single malformed block overflows the allocation. An
unauthenticated attacker fully controls both the allocation size and the copy
length/contents — a remotely triggerable heap overflow with attacker-controlled
data.

Affected Version

SCADACS/snap7 at commit f6ff90317ca5d54250f4dcd29209689a74e26d82
(src/core/s7_server.cpp, PerformFunctionDownload, lines 1145 / 1180-1188).

Reachability

REACHABLE, unauthenticated. Path: COTP connect → S7 pduReqDownload (0x1A) →
server acks and requests blocks → attacker replies with a malformed download
block. No registered area or prior auth required.

Technical Details

BlkLen = AsciiToNum(ReqStartParams->AsciiLoad, sizeof(ReqStartParams->AsciiLoad)); // attacker
pbyte buffer = new byte[BlkLen];                                     // line 1145
...
word DataLen = 0; size_t written = 0;
while (!last_packet && written < BlkLen) {
    isoSendBuffer(&Answer17, ...);
    isoRecvPDU(&PDU);
    ResHeader = PS7ResHeader23(pbyte(PDUH_in));
    DataLen = SwapWord(ResHeader->DataLen) - 4;                      // line 1184: word underflow
    memcpy(buffer + written, &pbyte(PDUH_in)[sizeof(TS7ResHeader23)+6], DataLen); // line 1185: OOB write
    last_packet = !(bool)(pbyte(PDUH_in)[sizeof(TS7ResHeader23)+1]);
    written += DataLen;
}

With BlkLen = 1 and a block whose header DataLen = 0, DataLen becomes
0xFFFC and the first memcpy writes 65532 bytes into a 1-byte allocation. The
copy also over-reads the ≤4096-byte input PDU as the source. isoRecvPDU's
return value is ignored, so a short/closed read does not stop the loop.

Data flow: TCP bytes → StartDownload AsciiLoad → new byte[BlkLen]; TCP bytes →
block header DataLen field → underflow → unbounded memcpy length → heap overflow.

Proof of Concept

poc/poc_snap7_download_heap.cpp starts the real TSnap7Server under ASan on
loopback, performs the COTP handshake, sends a StartDownload with AsciiLoad
"000001" (BlkLen = 1), drains the server's ack + block-request, then sends one
block PDU with S7 DataLen = 0. AddressSanitizer
(poc/crash/SNAP7-02_download_heap_asan.txt):

ERROR: AddressSanitizer: heap-buffer-overflow ... in __asan_memcpy
  #1 TS7Worker::PerformFunctionDownload() s7_server.cpp:1185
 allocated by thread T1 here:
  #0 operator new(unsigned long)

Impact

CVSS v3.1: 9.8 Critical — AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:H.
Unauthenticated remote heap overflow with attacker-controlled length and
contents; DoS and a plausible path to RCE.

Suggested Remediation

• Validate the block length against remaining capacity and the received PDU:
  if (rawDataLen < 4 || written + (rawDataLen-4) > BlkLen) { error; break; }
  computing in a width that cannot underflow (compare before subtracting 4).
• Check isoRecvPDU's return and abort on short/closed reads.
• Bound BlkLen to a sane maximum before new byte[BlkLen].