Pre-auth stack overflow in S7 Read handler (signed/unsigned guard bypass)

[entropy1337]

[CWE-787/CWE-190/CWE-121] Pre-auth stack overflow in S7 Read handler (signed/unsigned guard bypass) — snap7 server

Finding ID: SNAP7-01 Severity: High/Critical (unauthenticated remote)
Component: snap7 S7 server — TS7Worker::ReadArea() / PerformFunctionRead()

Summary

The per-item size guard in the S7 Read handler is written as
if (PDURemainder - Size <= 0), where PDURemainder is int and Size is
longword (uint32). By C++ integer-promotion rules the subtraction is evaluated
unsigned, so the <= 0 test can only ever be true when Size == PDURemainder
exactly; for Size > PDURemainder the result wraps to a large positive value and
the guard is bypassed. The guard exists to keep a read from exceeding the
negotiated PDU size (so the response fits the ~4 KB stack Answer buffer). With
the guard defeated, an attacker can request a read whose byte count is bounded
only by the size of a registered area — and the later Start + Size > ASize
check validates the source area, not the destination — so memcpy(&ResItemData->Data, Source, Size)
overflows the fixed-size Answer stack buffer.

Affected Version

SCADACS/snap7 at commit f6ff90317ca5d54250f4dcd29209689a74e26d82
(src/core/s7_server.cpp, TS7Worker::ReadArea).

Reachability

REACHABLE (unauthenticated), conditional on a registered data area larger
than the response buffer (~4 KB) — the normal case for an S7 server exposing an
8 KB / 32 KB / 64 KB DB. Path: COTP connect → S7 request pduFuncRead (0x04) →
PerformFunctionRead → ReadArea.

Technical Details

src/core/s7_server.cpp, ReadArea():

longword Start, Size, ASize, AStart;          // Size is unsigned 32-bit
...
Elements = SwapWord(ReqItemPar->Length);      // attacker u16, up to 65535
Size = Multiplier * Elements;                 // up to ~262140
if (PDURemainder - Size <= 0)                 // int - uint32 -> UNSIGNED compare (line 504)
    return RA_SizeOverPDU(...);               // only fires when Size == PDURemainder
else PDURemainder -= Size;
...
if (Start + Size > ASize)                     // bounds the SOURCE area only (line 548)
    return RA_OutOfRange(...);
Source = P->PData + Start;
...
memcpy(&ResItemData->Data, Source, Size);     // DEST is a slot in the 4 KB stack Answer (line 568)

ResItemData->Data points into TS7Answer23 Answer (a ~4096-byte stack buffer
in PerformFunctionRead). A read with Size between ~4 KB and the area size
overflows it.

Data flow: TCP bytes → item Length field → Size → broken guard → memcpy
length into ~4 KB stack buffer → stack overflow.

Proof of Concept

Reproduced with the AFL harness in file-replay mode against a server that
registers an 8 KB DB (poc/fuzz_s7_pdu.cpp, area srvAreaDB size 8192).
Crashing input poc/corpus/seed_read_overflow.bin (one read item, DB1, word-len
byte, Length = 0x1F40 = 8000). ASan (poc/crash/SNAP7-01_read_asan.txt):

ERROR: AddressSanitizer: stack-buffer-overflow in __asan_memcpy
  #1 TS7Worker::ReadArea(...)          s7_server.cpp:568
  #2 TS7Worker::PerformFunctionRead()  s7_server.cpp:654

Impact

CVSS v3.1: 9.1 — AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:H (High/Critical).
Unauthenticated remote stack overflow; reliable DoS, potential RCE. The
precondition (an exposed area larger than the answer buffer) is the standard
deployment.

Root Cause

Signed/unsigned mismatch in the size guard (int vs longword), plus reliance
on a source-area bound to protect a smaller destination buffer.

Suggested Remediation

Compare with matching signed types and against the destination capacity, e.g.:

if (Size > (longword)PDURemainder) return RA_SizeOverPDU(...);

and additionally bound Size by the response buffer capacity
(sizeof(Answer) - header/params), independent of the source-area check.