From 031b887e3623c6ff2488dfb9b8fcf8fc3e109351 Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Fri, 14 Aug 2026 16:21:41 +0000 Subject: [PATCH 1/3] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[HIGH]?= =?UTF-8?q?=20Fix=20readline=20integer=20coercion=20DoS=20vulnerability?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .jules/sentinel.md | 4 ++++ R/aFIPC.R | 6 +++--- tests/testthat/test-sentinel-validation.R | 18 ++++++++++++++++++ 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index a8207a48..92783659 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-07-16 - Fix readline integer coercion DoS vulnerability +**Vulnerability:** Weak regex `^[0-9]+$` for integer validation allows extremely large numeric strings, which coerce to `NA` via `as.integer()` and crash the R process when evaluated in `if` conditions. +**Learning:** When reading integer inputs via `readline()` in R, avoid weak regex validation. Large numbers coerce to `NA` breaking control flow logic. +**Prevention:** Use strictly bounded exact-match regex like `^[12]$` when prompting for specific integer options to prevent unexpected coercions and DoS vulnerabilities. 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..6d0875f2 100644 --- a/tests/testthat/test-sentinel-validation.R +++ b/tests/testthat/test-sentinel-validation.R @@ -35,3 +35,21 @@ test_that("autoFIPC validates boolean flags for newformBILOGprior, oldformBILOGp "Security Error: confirmCommonItems must be a single non-NA logical value or NULL" ) }) + +test_that("interactive readline input bounded regex validation prevents DoS", { + mockery::stub(aFIPC::autoFIPC, 'interactive', TRUE) + my_readline <- function(...) '99999999999999999999' + mockery::stub(aFIPC::autoFIPC, 'readline', my_readline) + + # This should error out from 'Too many invalid common item confirmation attempts' + # instead of failing due to NA coercion in if condition + expect_error( + aFIPC::autoFIPC( + newformXData = data.frame(A=1), + oldformYData = data.frame(A=2), + newformCommonItemNames = c('A'), + oldformCommonItemNames = c('A') + ), + "Too many invalid common item confirmation attempts" + ) +}) From 4f00b75ecb2217ce7515d2cd79b55ad23ff9c9eb Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 15 Aug 2026 01:38:31 +0900 Subject: [PATCH 2/3] test: cover BILOG prompts and refresh dependency cache --- .github/workflows/r.yml | 2 + tests/testthat/test-sentinel-validation.R | 68 +++++++++++++++++++++++ tests/testthat/test-workflow-cache.R | 27 +++++++++ 3 files changed, 97 insertions(+) create mode 100644 tests/testthat/test-workflow-cache.R diff --git a/.github/workflows/r.yml b/.github/workflows/r.yml index cf2e6561..869928ee 100644 --- a/.github/workflows/r.yml +++ b/.github/workflows/r.yml @@ -38,6 +38,8 @@ jobs: - name: Set up R package dependencies uses: r-lib/actions/setup-r-dependencies@d3c5be51b12e724e68f33216ca3c148b66d5f0b6 with: + # Refresh stale qs2/RcppParallel TBB ABI artifacts as one cache generation. + cache-version: '2' extra-packages: any::rcmdcheck needs: check diff --git a/tests/testthat/test-sentinel-validation.R b/tests/testthat/test-sentinel-validation.R index 6d0875f2..ac0b4638 100644 --- a/tests/testthat/test-sentinel-validation.R +++ b/tests/testthat/test-sentinel-validation.R @@ -53,3 +53,71 @@ test_that("interactive readline input bounded regex validation prevents DoS", { "Too many invalid common item confirmation attempts" ) }) + + +test_that("oldform BILOG prompt rejects repeated overlong integer input", { + mockery::stub(aFIPC::autoFIPC, "interactive", TRUE) + calls <- new.env(parent = emptyenv()) + calls$count <- 0L + overlong_readline <- function(...) { + calls$count <- calls$count + 1L + "99999999999999999999" + } + mockery::stub(aFIPC::autoFIPC, "readline", overlong_readline) + + expect_error( + aFIPC::autoFIPC( + newformXData = data.frame(A = 1), + oldformYData = data.frame(A = 2), + newformCommonItemNames = "A", + oldformCommonItemNames = "A", + newformBILOGprior = FALSE, + confirmCommonItems = TRUE + ), + "Too many invalid oldform BILOG prior attempts" + ) + expect_equal(calls$count, 3L) +}) + +test_that("newform BILOG prompt rejects repeated overlong integer input", { + skip_if_not_installed("mirt") + set.seed(20260815) + old_data <- as.data.frame(mirt::simdata( + a = matrix(rep(1, 4), ncol = 1), + d = c(-1, -0.3, 0.3, 1), + itemtype = rep("2PL", 4), + N = 200 + )) + names(old_data) <- LETTERS[1:4] + old_model <- mirt::mirt( + old_data, + 1, + itemtype = "2PL", + SE = FALSE, + verbose = FALSE + ) + + mockery::stub(aFIPC::autoFIPC, "interactive", TRUE) + calls <- new.env(parent = emptyenv()) + calls$count <- 0L + overlong_readline <- function(...) { + calls$count <- calls$count + 1L + "99999999999999999999" + } + mockery::stub(aFIPC::autoFIPC, "readline", overlong_readline) + + expect_error( + aFIPC::autoFIPC( + newformXData = old_data, + oldformYData = old_model, + newformCommonItemNames = "A", + oldformCommonItemNames = "A", + itemtype = "3PL", + oldformBILOGprior = FALSE, + confirmCommonItems = TRUE, + checkIPD = FALSE + ), + "Too many invalid newform BILOG prior attempts" + ) + expect_equal(calls$count, 3L) +}) diff --git a/tests/testthat/test-workflow-cache.R b/tests/testthat/test-workflow-cache.R new file mode 100644 index 00000000..7ea7c4f4 --- /dev/null +++ b/tests/testthat/test-workflow-cache.R @@ -0,0 +1,27 @@ +test_that("R CMD check uses an active reviewed dependency cache generation", { + workflow_path <- testthat::test_path( + "..", "..", ".github", "workflows", "r.yml" + ) + workflow_lines <- readLines(workflow_path, warn = FALSE) + dependency_step <- grep( + "r-lib/actions/setup-r-dependencies@", + workflow_lines, + fixed = TRUE + ) + expect_length(dependency_step, 1L) + dependency_block <- workflow_lines[ + dependency_step:min(dependency_step + 8L, length(workflow_lines)) + ] + active_cache_version_pattern <- paste0( + "^[[:space:]]*cache-version:[[:space:]]*", + "['\\\"]2['\\\"][[:space:]]*(#.*)?$" + ) + + expect_false( + grepl(active_cache_version_pattern, "# cache-version: '2'", perl = TRUE) + ) + expect_true( + any(grepl(active_cache_version_pattern, dependency_block, perl = TRUE)), + info = "The TBB ABI cache refresh must remain an active workflow input" + ) +}) From 46b9419d1d3cadd38127fd82dda899a275b167eb Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 15 Aug 2026 01:53:28 +0900 Subject: [PATCH 3/3] test(ci): install helpers and locate workflow from checks --- DESCRIPTION | 4 +++- tests/testthat/test-workflow-cache.R | 6 ++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index f31d3e1a..5742bef1 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -10,7 +10,9 @@ 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: + mockery, + testthat (>= 3.0.0) Encoding: UTF-8 Config/testthat/edition: 3 Config/roxygen2/version: 8.0.0 diff --git a/tests/testthat/test-workflow-cache.R b/tests/testthat/test-workflow-cache.R index 7ea7c4f4..951836ec 100644 --- a/tests/testthat/test-workflow-cache.R +++ b/tests/testthat/test-workflow-cache.R @@ -1,7 +1,9 @@ test_that("R CMD check uses an active reviewed dependency cache generation", { - workflow_path <- testthat::test_path( - "..", "..", ".github", "workflows", "r.yml" + repository_root <- Sys.getenv( + "GITHUB_WORKSPACE", + unset = testthat::test_path("..", "..") ) + workflow_path <- file.path(repository_root, ".github", "workflows", "r.yml") workflow_lines <- readLines(workflow_path, warn = FALSE) dependency_step <- grep( "r-lib/actions/setup-r-dependencies@",