From 7ad3034985211040b5d4c91ea02835c9132bd5f0 Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Sun, 2 Aug 2026 16:25:23 +0000 Subject: [PATCH 1/4] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[MEDIUM?= =?UTF-8?q?]=20Fix=20integer=20coercion=20DoS=20vulnerability=20in=20readl?= =?UTF-8?q?ine=20validation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaced weak regex validation `^[0-9]+$` with strictly bounded exact-match `^[12]$` across all interactive `readline()` prompts in `R/aFIPC.R`. This prevents large numeric strings from coercing to `NA` via `as.integer()`, which causes unhandled runtime crashes when evaluated in conditionals. Also added tests to verify correct input validation and rejection behavior using `mockery`. --- .jules/sentinel.md | 5 +++ R/aFIPC.R | 6 ++-- tests/testthat/test-sentinel-validation.R | 43 +++++++++++++++++++++++ 3 files changed, 51 insertions(+), 3 deletions(-) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index a8207a4..7d5925e 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -2,3 +2,8 @@ **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-02 - Fix integer coercion DoS vulnerability +**Vulnerability:** Weak regex `^[0-9]+$` on interactive `readline()` user inputs allows huge numeric strings which coerce to `NA` when passed to `as.integer()`. +**Learning:** In R, evaluating `NA` inside an `if()` condition or returning `NA` from a function expected to return integers can break program logic or crash automation processes. +**Prevention:** Use strictly bounded exact-match regex like `^[12]$` when validating integer inputs intended to be explicitly mapped to fixed choices. diff --git a/R/aFIPC.R b/R/aFIPC.R index 6254651..918e19b 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 900f0ee..dafde55 100644 --- a/tests/testthat/test-sentinel-validation.R +++ b/tests/testthat/test-sentinel-validation.R @@ -35,3 +35,46 @@ 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 integer validation works with strictly bounded regex", { + # Mock readline to return invalid inputs that should fail after 3 attempts + # override interactive() to TRUE to trigger the readline loop, then mock readline + with_mockery <- function() { + mockery::stub(aFIPC::autoFIPC, 'interactive', TRUE) + mockery::stub(aFIPC::autoFIPC, 'readline', mockery::mock('3', '0', '999999999')) + expect_error( + aFIPC::autoFIPC( + newformXData = data.frame(A=1, B=2), + oldformYData = data.frame(A=1, B=2), + newformCommonItemNames = c('A', 'B'), + oldformCommonItemNames = c('A', 'B'), + confirmCommonItems = NULL + ), + "Too many invalid common item confirmation attempts" + ) + } + with_mockery() +}) + +test_that("autoFIPC integer validation accepts valid inputs", { + # Mock readline to return '1' (valid input) + with_mockery_valid <- function() { + mockery::stub(aFIPC::autoFIPC, 'interactive', TRUE) + mockery::stub(aFIPC::autoFIPC, 'readline', '1') + mockery::stub(aFIPC::autoFIPC, 'mirt::mirt', function(...) stop('forced failure')) + mockery::stub(aFIPC::autoFIPC, 'surveyFA', function(...) stop('forced failure')) + + expect_error( + aFIPC::autoFIPC( + newformXData = data.frame(A=1, B=2), + oldformYData = data.frame(A=1, B=2), + newformCommonItemNames = c('A', 'B'), + oldformCommonItemNames = c('A', 'B'), + confirmCommonItems = NULL, + oldformBILOGprior = NULL, + newformBILOGprior = NULL + ) + ) + } + with_mockery_valid() +}) From dfeb9b3ff1d4241c7f80bcda8c564f59c2049672 Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Sun, 2 Aug 2026 16:50:49 +0000 Subject: [PATCH 2/4] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[MEDIUM?= =?UTF-8?q?]=20Fix=20integer=20coercion=20DoS=20vulnerability=20in=20readl?= =?UTF-8?q?ine=20validation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaced weak regex validation `^[0-9]+$` with strictly bounded exact-match `^[12]$` across all interactive `readline()` prompts in `R/aFIPC.R`. This prevents large numeric strings from coercing to `NA` via `as.integer()`, which causes unhandled runtime crashes when evaluated in conditionals. Also added tests to verify correct input validation and rejection behavior using `mockery`. Also fixed R CMD check warning by adding `.semgrepignore` to `.Rbuildignore`. --- .Rbuildignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.Rbuildignore b/.Rbuildignore index 232504f..388f1c6 100644 --- a/.Rbuildignore +++ b/.Rbuildignore @@ -22,3 +22,4 @@ ^\.jules(/.*)?$ ^\.trivyignore\.yaml$ ^trivy\.yaml$ +^\.semgrepignore$ From 6cf12706cc9ef8c16ba3e32b60e18d4c183d992e Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Sun, 2 Aug 2026 17:18:28 +0000 Subject: [PATCH 3/4] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[MEDIUM?= =?UTF-8?q?]=20Fix=20integer=20coercion=20DoS=20vulnerability=20in=20readl?= =?UTF-8?q?ine=20validation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaced weak regex validation ^[0-9]+$ with strictly bounded exact-match ^[12]$ across all interactive readline() prompts in R/aFIPC.R. This prevents large numeric strings from coercing to NA via as.integer(), which causes unhandled runtime crashes when evaluated in conditionals. Also added tests to verify correct input validation and rejection behavior using mockery. Also fixed R CMD check warning by adding .semgrepignore to .Rbuildignore. From f67f277b53d6b32c07408c238dba765c673a07ea Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Sun, 2 Aug 2026 17:33:24 +0000 Subject: [PATCH 4/4] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[MEDIUM?= =?UTF-8?q?]=20Fix=20integer=20coercion=20DoS=20vulnerability=20in=20readl?= =?UTF-8?q?ine=20validation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaced weak regex validation ^[0-9]+$ with strictly bounded exact-match ^[12]$ across all interactive readline() prompts in R/aFIPC.R. This prevents large numeric strings from coercing to NA via as.integer(), which causes unhandled runtime crashes when evaluated in conditionals. Also added tests to verify correct input validation and rejection behavior using mockery. Also fixed R CMD check warnings by adding .semgrepignore to .Rbuildignore, removing top-level dummy files, and declaring test dependencies. --- DESCRIPTION | 2 +- test_dummy.R | 2 -- test_validation.R | 3 --- 3 files changed, 1 insertion(+), 6 deletions(-) delete mode 100644 test_dummy.R delete mode 100644 test_validation.R diff --git a/DESCRIPTION b/DESCRIPTION index f31d3e1..c90753c 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 diff --git a/test_dummy.R b/test_dummy.R deleted file mode 100644 index e6f7019..0000000 --- a/test_dummy.R +++ /dev/null @@ -1,2 +0,0 @@ -source("R/aFIPC.R") -source("R/surveyFA.R") diff --git a/test_validation.R b/test_validation.R deleted file mode 100644 index f084116..0000000 --- a/test_validation.R +++ /dev/null @@ -1,3 +0,0 @@ -source("R/aFIPC.R") -source("R/surveyFA.R") -print("Syntax check passed")