From 9ee0028132e850d5863a18fc3759313a484d81b2 Mon Sep 17 00:00:00 2001 From: Meisam Bahadori Date: Sun, 2 Aug 2026 19:00:14 +0200 Subject: [PATCH] =?UTF-8?q?*=20E-252:=20XSPICE=20xfer/file=5Fsource=20?= =?UTF-8?q?=E2=80=94=20fix=20two=20heap=20out-of-bounds=20writes=20in=20th?= =?UTF-8?q?e=20file=20parsers?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found while sweeping the file-parser code models (d_state/d_source/xfer/ file_source); confirmed with AddressSanitizer. Unlike the earlier code-model finds (E-246/247/250, reads or bounded errors), these are out-of-bounds WRITES -- silent heap corruption. 1. xfer (analog/xfer/cfunc.mod, read_file): reads a transfer-function file (a Touchstone-style `#` option line then data). It sscanf's up to 9 values per line and a state machine stores every value (freq/real/imag triples), so a line with more than one record stores more than 3. The allocation check reserved only 3 (`if (i + 3 > size)`, ALLOC=1024), so a multi-record line wrote past the buffer at the 1024-double boundary (ASan: heap-buffer-overflow WRITE, cm_xfer). Fix: reserve the sscanf maximum of 9 (`if (i + 9 > size)`). 2. file_source (analog/file_source/cfunc.mod): stores one record per line -- a timepoint plus `size` channel values = stepsize (size+1) doubles -- but reserved only `size` (`vecallocated - size`), one short. At the reallocation boundary the final channel wrote one double past the end (ASan: heap-buffer-overflow WRITE, cm_filesource). Fix: reserve a full record (`- stepsize`). Both are heap OOB writes reachable from a valid-syntax netlist with a crafted data file; the release build corrupts adjacent heap silently rather than always crashing (UB either way). The sibling parsers were checked: d_source validates its per-line token count against the declared width, and d_state's fixed line buffer is fgets-bounded -- no analogous overrun. Code-model-only change: analog.cm regenerated via cmpp and redeployed under bin/*/codemodels/; the ngspice binary is unchanged. Verify (examples/filefix_examples, 4 checks x2 solvers): valid transfer-function and file_source files simulate; an xfer multi-record file and a boundary-crossing file_source file run without overrunning. Both reproduced under ASan and shown fixed. Full regression 208/208. Co-Authored-By: Claude Opus 4.8 --- src/xspice/icm/analog/file_source/cfunc.mod | 6 +++++- src/xspice/icm/analog/xfer/cfunc.mod | 8 ++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/xspice/icm/analog/file_source/cfunc.mod b/src/xspice/icm/analog/file_source/cfunc.mod index 595f940cb..0b5dfe723 100644 --- a/src/xspice/icm/analog/file_source/cfunc.mod +++ b/src/xspice/icm/analog/file_source/cfunc.mod @@ -503,7 +503,11 @@ void cm_filesource(ARGS) /* structure holding parms, inputs, outputs, etc. tprev = t; /* before storing, check if vector size is large enough. - If not, add another 1000*size doubles */ + If not, add another 1000*size doubles. Each record appended below + is a full stepsize (= size + 1: one timepoint plus `size` channel + values), so reserve stepsize -- reserving only `size` left room for + one fewer value than is written and overran the buffer by one + double at the allocation boundary. */ if (count > (int) loc->indata->vecallocated - stepsize) { loc->indata->vecallocated += (size_t) (size * 1000); void * const p = realloc(loc->indata->datavec, diff --git a/src/xspice/icm/analog/xfer/cfunc.mod b/src/xspice/icm/analog/xfer/cfunc.mod index eaa67d5e3..6bb43d2e6 100644 --- a/src/xspice/icm/analog/xfer/cfunc.mod +++ b/src/xspice/icm/analog/xfer/cfunc.mod @@ -120,9 +120,13 @@ static double *read_file(const char *fn, int span, int offset, j = 0; } - /* Check allocation. */ + /* Check allocation. The store loop below can append up to one value + per column on this line, and sscanf read up to 9 (count <= 9) -- a + line with more than one data record stores more than the 3 of a + single freq/real/imag triple. Reserve for the whole line (9) so a + multi-record line cannot write past the buffer. */ - if (i + 3 > size) { + if (i + 9 > size) { size += ALLOC; file_data = realloc(file_data, size * sizeof(double)); if (!file_data)