Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .jules/sentinel.md
Original file line number Diff line number Diff line change
Expand Up @@ -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-06 - Fix integer overflow coercion vulnerability in interactive inputs
**Vulnerability:** Integer overflow coercion vulnerability where excessively large numeric strings pass unbounded regex checks (e.g., `^[0-9]+$`) but evaluate to `NA` in `as.integer()`, causing subsequent process crashes.
**Learning:** In R scripts, when validating interactive `readline()` numeric inputs, unbounded digit classes (e.g., `^[0-9]+$`) can lead to unexpected `NA` coercion for extremely large numbers, leading to unhandled downstream exceptions.
**Prevention:** Strictly match interactive numeric inputs against exact expected values (e.g., `^[12]$`) rather than unbounded digit classes.
6 changes: 3 additions & 3 deletions R/aFIPC.R
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
}
Expand Down Expand Up @@ -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))
}
}
Expand Down Expand Up @@ -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))
}
}
Expand Down
31 changes: 31 additions & 0 deletions tests/testthat/test-sentinel-validation.R
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,34 @@ 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 properly validates interactive readline inputs to prevent integer overflow coercion", {
# Mock interactive() to return TRUE
mock_interactive <- mockery::mock(TRUE, cycle = TRUE)

# Mock readline for confirmCommonItems (reject invalid, then valid)
mock_readline_confirm <- mockery::mock("3", "999999999999999999999999", "1", cycle = TRUE)

# Stub both functions in the aFIPC namespace
mockery::stub(aFIPC::autoFIPC, 'interactive', mock_interactive)
mockery::stub(aFIPC::autoFIPC, 'readline', mock_readline_confirm)

# We expect an error down the line because this is a dummy data frame, but
# we want to ensure it passes the readline block and doesn't get stuck or crash due to coercion
# We'll test oldformBILOGprior similarly by passing the confirm step

mock_readline_oldform <- mockery::mock("1", "4", "99999999999999999", "2", cycle = TRUE)
mockery::stub(aFIPC::autoFIPC, 'readline', mock_readline_oldform)

expect_error(
aFIPC::autoFIPC(
newformXData = data.frame(A=c(1, 2, 1, 0, 1)),
oldformYData = data.frame(A=c(1, 0, 1, 0, 1)),
newformCommonItemNames = c('A'),
oldformCommonItemNames = c('A'),
confirmCommonItems = NULL,
itemtype = '3PL'
),
"Initial estimation of oldFormModel completely failed"
)
})