From 673674d1f9ca06da3702e1461a9fe892de2135f3 Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Sun, 16 Aug 2026 16:27:16 +0000 Subject: [PATCH 1/2] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[CRITIC?= =?UTF-8?q?AL]=20Fix=20integer=20coercion=20DoS=20in=20readline=20prompts?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit addresses a critical DoS vulnerability where excessively large string inputs to `readline` prompts were coercing to `NA` through `as.integer()`, leading to unhandled exceptions downstream. The fix replaces the weak `^[0-9]+$` regex with a strictly bounded exact-match `^[12]$` regex. Also includes rigorous mocked test cases. --- .jules/sentinel.md | 4 ++ R/aFIPC.R | 6 +- tests/testthat/test-sentinel-validation.R | 69 +++++++++++++++++++++++ 3 files changed, 76 insertions(+), 3 deletions(-) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index a8207a48..7a18fa05 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -2,3 +2,7 @@ **Vulnerability:** Unvalidated inputs passed to `if()` statements can cause process crashes (`condition has length > 1`) or unexpected coercion vulnerabilities. **Learning:** In R, optional boolean parameters that default to `NULL` should be validated using explicit runtime type validation (e.g., `if (!is.null(flag) && (!is.logical(flag) || length(flag) != 1 || is.na(flag)))`). **Prevention:** Always implement explicit runtime type validation for optional boolean parameters. +## 2024-08-01 - Fix integer coercion DoS vulnerability in readline prompts +**Vulnerability:** Weak regex `^[0-9]+$` on interactive `readline()` inputs allowed excessively large strings (e.g. "9999999999") to pass validation, which when coerced by `as.integer()` returned `NA`. This caused unhandled exceptions in subsequent `if` conditions, leading to unexpected crashes (Denial of Service). +**Learning:** Base `as.integer()` silently coerces out-of-bounds numeric strings to `NA` with a warning, bypassing simple digit-only regex checks. +**Prevention:** Use strictly bounded exact-match regex (like `^[12]$`) when validating interactive menu choices to guarantee safe integer coercion and prevent runtime crashes. diff --git a/R/aFIPC.R b/R/aFIPC.R index 62546519..918e19b1 100644 --- a/R/aFIPC.R +++ b/R/aFIPC.R @@ -141,7 +141,7 @@ autoFIPC <- } for (attempt in seq_len(3)) { n <- readline(prompt = "Is it correct? (1: Yes 2: No) : ") - if (grepl("^[0-9]+$", n)) { + if (grepl("^[12]$", n)) { return(as.integer(n)) } } @@ -171,7 +171,7 @@ autoFIPC <- readline( prompt = "Do you want to use default BILOG-MG priors for oldform Data? (1: Yes 2: No) : " ) - if (grepl("^[0-9]+$", n)) { + if (grepl("^[12]$", n)) { return(as.integer(n)) } } @@ -390,7 +390,7 @@ autoFIPC <- readline( prompt = "Do you want to use default BILOG-MG priors for newform Data? (1: Yes 2: No) : " ) - if (grepl("^[0-9]+$", n)) { + if (grepl("^[12]$", n)) { return(as.integer(n)) } } diff --git a/tests/testthat/test-sentinel-validation.R b/tests/testthat/test-sentinel-validation.R index 900f0ee3..e42a08f8 100644 --- a/tests/testthat/test-sentinel-validation.R +++ b/tests/testthat/test-sentinel-validation.R @@ -35,3 +35,72 @@ test_that("autoFIPC validates boolean flags for newformBILOGprior, oldformBILOGp "Security Error: confirmCommonItems must be a single non-NA logical value or NULL" ) }) + +test_that("autoFIPC interactive prompts prevent NA coercion crashes and DoS vulnerabilities from large integer inputs", { + + # verify oldformBILOGprior validation protects against large integer inputs + + mockery::stub(aFIPC::autoFIPC, 'interactive', function(...) TRUE) + mockery::stub(aFIPC::autoFIPC, 'readline', function(...) "9999999999") + + expect_error( + aFIPC::autoFIPC( + newformXData = data.frame(A=1), + oldformYData = data.frame(A=2), + newformCommonItemNames = c('A'), + oldformCommonItemNames = c('A'), + confirmCommonItems = TRUE, + itemtype = '3PL', + tryFitwholeNewItems = FALSE, + tryFitwholeOldItems = FALSE, + checkIPD = FALSE + ), + "Too many invalid oldform BILOG prior attempts" + ) + + # verify newformBILOGprior validation protects against large integer inputs + mockery::stub(aFIPC::autoFIPC, 'interactive', function(...) TRUE) + mockery::stub(aFIPC::autoFIPC, 'readline', function(...) "9999999999") + + # We need oldformYData to be a model or at least have multiple categories so it bypasses failure during mirt fitting, + # or we can mock mirt::mirt so it returns a dummy model to reach newformBILOGprior check. + mod <- new("SingleGroupClass") + mod@OptimInfo$converged <- TRUE + mod@OptimInfo$secondordertest <- TRUE + mod@Data$data <- data.frame(A=c(1,0)) + mockery::stub(aFIPC::autoFIPC, 'mirt::mirt', function(...) mod) + mockery::stub(aFIPC::autoFIPC, 'surveyFA', function(...) mod) + + expect_error( + aFIPC::autoFIPC( + newformXData = data.frame(A=1), + oldformYData = data.frame(A=c(1,0)), + newformCommonItemNames = c('A'), + oldformCommonItemNames = c('A'), + confirmCommonItems = TRUE, + oldformBILOGprior = TRUE, + itemtype = '3PL', + tryFitwholeNewItems = FALSE, + tryFitwholeOldItems = FALSE, + checkIPD = FALSE + ), + "Too many invalid newform BILOG prior attempts" + ) + + # verify confirmCommonItems validation protects against large integer inputs + mockery::stub(aFIPC::autoFIPC, 'interactive', function(...) TRUE) + mockery::stub(aFIPC::autoFIPC, 'readline', function(...) "9999999999") + expect_error( + aFIPC::autoFIPC( + newformXData = data.frame(A=1), + oldformYData = data.frame(A=2), + newformCommonItemNames = c('A'), + oldformCommonItemNames = c('A'), + itemtype = '3PL', + tryFitwholeNewItems = FALSE, + tryFitwholeOldItems = FALSE, + checkIPD = FALSE + ), + "Too many invalid common item confirmation attempts" + ) +}) From 8cf1dedfc4444c9ebc03dce6ca7a2e797d7df655 Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Sun, 16 Aug 2026 19:42:51 +0000 Subject: [PATCH 2/2] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[CRITIC?= =?UTF-8?q?AL]=20Fix=20integer=20coercion=20DoS=20in=20readline=20prompts?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit addresses a critical DoS vulnerability where excessively large string inputs to `readline` prompts were coercing to `NA` through `as.integer()`, leading to unhandled exceptions downstream. The fix replaces the weak `^[0-9]+$` regex with a strictly bounded exact-match `^[12]$` regex. Also includes rigorous mocked test cases. --- DESCRIPTION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DESCRIPTION b/DESCRIPTION index f31d3e1a..c90753c5 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -10,7 +10,7 @@ Description: Automates fixed item parameter linking for test linking under the item response theory paradigm using mirt package estimates. License: GPL-3 | file LICENSE Imports: mirt, methods -Suggests: testthat (>= 3.0.0) +Suggests: testthat (>= 3.0.0), mockery Encoding: UTF-8 Config/testthat/edition: 3 Config/roxygen2/version: 8.0.0