From 9e07d2d43d2b14c205e6fc75e7a1c36f137286bc Mon Sep 17 00:00:00 2001 From: MarkLee131 Date: Wed, 29 Apr 2026 22:51:54 +0800 Subject: [PATCH] skel: validate yytables_fload header buffer and bound table dimensions The binary tables-file loader trusts attacker-controlled bytes from the file in three places. Fix all three: 1. yytbl_hdr_read at cpp-flex.skl:4010 did th->th_name = th->th_version + strlen(th->th_version) + 1; immediately after fread'ing th_hsize - 14 bytes into th_version. If those bytes contain no NUL, strlen walks past the heap region. Fix: use strnlen() and require both the version and the name string to terminate with NULs inside the buffer; return -1 otherwise. 2. yytbl_fload at cpp-flex.skl:4278 then did if (strcmp(th.th_name, key) != 0) even when th_name == th_version + bytes, where the buffer has no room for the name section. Fixed by the same validation in (1): th_name is only set when a name NUL is found inside the buffer. 3. yytbl_data_load at cpp-flex.skl:4090-4092 computes bytes = sizeof(yy_trans_info) * td_lolen * td_hilen (or the equivalent non-STRUCT form) with td_lolen and td_hilen read directly from the file as flex_uint32_t. A 50-byte tables file with td_lolen near UINT32_MAX makes the loader call yyalloc(8 GB). Fix: reject td_lolen or td_hilen above YYTBL_MAX_DIM (1<<24) before the multiplication, mirroring 9c54eb6 which added the analogous guard to the build-time tool's allocators. Verified with three minimum-size PoCs (16, 16, 50 bytes) that the unpatched loader trips ASAN on each, and the patched loader returns -1 cleanly. A valid tables file generated by `flex --tables-file=` still loads as before (regression check passed). Found via a libFuzzer harness against yytables_fload, expanding what the upstream OSS-Fuzz projects/flex integration covers (their flex-patch.diff neuters the m4 fork, which keeps the loader off the fuzzed code path). --- src/cpp-flex.skl | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/src/cpp-flex.skl b/src/cpp-flex.skl index 9610b2019..f81f16a7c 100644 --- a/src/cpp-flex.skl +++ b/src/cpp-flex.skl @@ -4007,7 +4007,27 @@ static int yytbl_hdr_read YYFARGS2(struct yytbl_hdr *, th, struct yytbl_reader * } else { rd->bread += (flex_uint32_t) bytes; } - th->th_name = th->th_version + strlen (th->th_version) + 1; + /* Validate that the buffer contains TWO NUL-terminated strings within + * its bounds (a version string followed by a name string). Without + * these checks, a header whose payload is missing either NUL would + * cause the strlen() below, and the strcmp() in yytbl_fload, to read + * past the heap allocation. */ + { + size_t vlen, nlen; + vlen = strnlen (th->th_version, bytes); + if (vlen >= bytes - 1) { + yyfree (th->th_version M4_YY_CALL_LAST_ARG); + th->th_version = NULL; + return -1; + } + nlen = strnlen (th->th_version + vlen + 1, bytes - vlen - 1); + if (nlen >= bytes - vlen - 1) { + yyfree (th->th_version M4_YY_CALL_LAST_ARG); + th->th_version = NULL; + return -1; + } + th->th_name = th->th_version + vlen + 1; + } return 0; } @@ -4047,6 +4067,19 @@ static int yytbl_data_load YYFARGS2(struct yytbl_dmap *, dmap, struct yytbl_read || yytbl_read32 (&td.td_lolen, rd) != 0) { return -1; } + /* Bound the dimensions before any size_t multiplication. Without + * this, a malicious tables file with td_lolen and td_hilen set to + * arbitrary uint32 values drives the bytes computation below into + * a multi-GB allocation request. YYTBL_MAX_DIM is well above what + * any real scanner produces (the documented NFA-state limit is + * around 32k; tables larger than a few million elements would not + * fit any realistic memory budget). */ +#ifndef YYTBL_MAX_DIM +#define YYTBL_MAX_DIM (1u << 24) +#endif + if (td.td_lolen > YYTBL_MAX_DIM || td.td_hilen > YYTBL_MAX_DIM) { + return -1; + } /* Lookup the map for the transition table so we have it in case we need it * inside the loop below. This scanner might not even have a transition * table, which is ok.