From 0bc5057ce18e2e09feb174f90a6e50fd8e1b5e1f Mon Sep 17 00:00:00 2001 From: Bill Denney Date: Fri, 1 May 2026 20:42:33 +0000 Subject: [PATCH 1/2] Guard sbuf size calculations against int overflow `sAppendN`/`sAppend`/`addLine` (s and lProp paths) compute the new allocation size as int mx = sbb->o + 2 + n + SBUF_MXBUF; where `n` is a user-controlled length. When `n` is large enough the addition overflows `int` to a negative value; `R_Realloc` then interprets that as a huge unsigned size and crashes (or fails) instead of returning an informative R error. Add a check `n > INT_MAX - ` immediately before each allocation expression, raising `string buffer size overflow: input too large` (and `line array size overflow: too many lines` for the lProp path) before the unsigned wrap can happen. Mirror of the fix in rxode2 inst/include/sbuf.c. monolix2rx ships its own copy of the same buffer routines so it is fixed independently. Co-Authored-By: Claude Opus 4.7 (1M context) --- NEWS.md | 10 ++++++++++ src/sbuf.c | 12 ++++++++++++ 2 files changed, 22 insertions(+) diff --git a/NEWS.md b/NEWS.md index 2f38cf6..c85a672 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,13 @@ +# monolix2rx 0.0.7 + +* Add integer overflow guards in the C-level string buffer + (`src/sbuf.c`). `sAppendN`, `sAppend`, and `addLine` previously + computed the new allocation size as `sbb->o + 2 + n + SBUF_MXBUF` + (or analogous expression). When the user-controlled `n` was large + enough this expression overflowed `int` to a negative value, which + `R_Realloc` then converted to a huge unsigned size and crashed. The + guard converts this into a clean R error. + # monolix2rx 0.0.6 * Updated to add types for rstudio completion diff --git a/src/sbuf.c b/src/sbuf.c index 9eba1be..59e39b1 100644 --- a/src/sbuf.c +++ b/src/sbuf.c @@ -28,6 +28,9 @@ void sFreeIni(sbuf *sbb) { void sAppendN(sbuf *sbb, const char *what, int n) { if (sbb->sN == 0) sIni(sbb); if (sbb->sN <= 2 + n + sbb->o){ + if (n > INT_MAX - sbb->o - 2 - SBUF_MXBUF) { + Rf_error("string buffer size overflow: input too large"); + } int mx = sbb->o + 2 + n + SBUF_MXBUF; sbb->s = R_Realloc(sbb->s, mx, char); sbb->sN = mx; @@ -52,6 +55,9 @@ void sAppend(sbuf *sbb, const char *format, ...) { #endif va_end(copy); if (sbb->sN <= sbb->o + n + 1) { + if (n > INT_MAX - sbb->o - 1 - SBUF_MXBUF) { + Rf_error("string buffer size overflow: input too large"); + } int mx = sbb->o + n + 1 + SBUF_MXBUF; sbb->s = R_Realloc(sbb->s, mx, char); sbb->sN = mx; @@ -110,6 +116,9 @@ void addLine(vLines *sbb, const char *format, ...) { } va_end(copy); if (sbb->sN <= sbb->o + n){ + if (n > INT_MAX - sbb->sN - 2 - SBUF_MXBUF) { + Rf_error("string buffer size overflow: input too large"); + } int mx = sbb->sN + n + 2 + SBUF_MXBUF; sbb->s = R_Realloc(sbb->s, mx, char); // The sbb->line are not correct any longer because the pointer for sbb->s has been updated; @@ -122,6 +131,9 @@ void addLine(vLines *sbb, const char *format, ...) { vsnprintf(sbb->s + sbb->o, sbb->sN - sbb->o, format, argptr); va_end(argptr); if (sbb->n + 2 >= sbb->nL){ + if (n > INT_MAX - sbb->nL - 2 - SBUF_MXLINE) { + Rf_error("line array size overflow: too many lines"); + } int mx = sbb->nL + n + 2 + SBUF_MXLINE; sbb->lProp = R_Realloc(sbb->lProp, mx, int); sbb->lType = R_Realloc(sbb->lType, mx, int); From 93bf526043c4d5742ebbd32181b4fdff792bc0dc Mon Sep 17 00:00:00 2001 From: Bill Denney Date: Fri, 1 May 2026 21:14:55 +0000 Subject: [PATCH 2/2] Add regression test for sbuf int-overflow guard Skipped by default because triggering the overflow needs a ~2.147 GB identifier string (~2.5 GB peak RAM during the test). Mirrors the test added to rxode2 and nonmem2rx for the same fix. Manual-run instructions are in the test body. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/testthat/test-mem-sbuf-overflow.R | 48 +++++++++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 tests/testthat/test-mem-sbuf-overflow.R diff --git a/tests/testthat/test-mem-sbuf-overflow.R b/tests/testthat/test-mem-sbuf-overflow.R new file mode 100644 index 0000000..6d0eb01 --- /dev/null +++ b/tests/testthat/test-mem-sbuf-overflow.R @@ -0,0 +1,48 @@ +test_that("sbuf overflow guard triggers a graceful error (skipped: requires ~2.5GB RAM)", { + skip("Requires ~2.5GB RAM to construct a 2.147 GB identifier string that triggers integer overflow in buffer size calculation") + # --- What this test checks --- + # `sAppendN`/`sAppend`/`addLine` in src/sbuf.c computed the new allocation + # size as: + # int mx = sbb->o + 2 + n + SBUF_MXBUF; + # When the user-controlled `n` exceeds `INT_MAX - sbb->o - 2 - SBUF_MXBUF`, + # this expression overflows `int` to a negative value. R_Realloc then + # converts it to a huge `size_t` and either crashes (older R) or fails the + # allocation cleanly (modern R). Either way it is a memory bug rather than + # a controlled error path. + # + # The overflow guard added by this fix checks BEFORE the calculation: + # if (n > INT_MAX - sbb->o - 2 - SBUF_MXBUF) Rf_error(...) + # so the crash is replaced by a controlled R error. + # + # --- How to run manually (outside devtools::test()) --- + # Start a fresh R session with at least 3 GB of available RAM, then: + # + # library(monolix2rx) + # big <- strrep("x", 2147435646L) # ~2.147 GB + # src <- sprintf("[LONGITUDINAL]\ninput={%s}\nEQUATION:\nf=%s\n", big, big) + # # Before fix: SIGSEGV from R_Realloc with negative size + # # After fix: error "string buffer size overflow: input too large" + # try(.Call(`_monolix2rx_trans_equation`, src, "rxode2")) + + big <- strrep("x", 2147435646L) + src <- sprintf("[LONGITUDINAL]\ninput={%s}\nEQUATION:\nf=%s\n", big, big) + expect_error( + .Call(`_monolix2rx_trans_equation`, src, "rxode2"), + "string buffer size overflow|too large" + ) +}) + +test_that("sbuf handles moderately large inputs without error", { + # Sanity check: the overflow guard must not trigger on realistic inputs. + large_input <- paste(rep("a=1\n", 1e4), collapse = "") + expect_no_error( + tryCatch( + .Call(`_monolix2rx_trans_equation`, large_input, "rxode2"), + error = function(e) { + if (grepl("buffer size overflow", conditionMessage(e))) stop(e) + # Any other parse error is acceptable for this malformed input. + NULL + } + ) + ) +})