Skip to content
Closed
Show file tree
Hide file tree
Changes from 4 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
1 change: 1 addition & 0 deletions .Rbuildignore
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,4 @@
^\.jules(/.*)?$
^\.trivyignore\.yaml$
^trivy\.yaml$
^\.semgrepignore$
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-07-31 - Fix Denial of Service in readline integer coercions
**Vulnerability:** Weak regex `^[0-9]+$` on interactive `readline()` user inputs allows abnormally large integers that will coerce to `NA` when evaluated via `as.integer()`. This can cause unhandled exceptions and infinite loops/crashes when the `NA` value is evaluated in downstream logic.
**Learning:** In R, strings representing integers larger than `.Machine$integer.max` coerce to `NA`.
**Prevention:** Use strictly bounded exact-match regex validations (e.g., `^[12]$`) for expected input formats to prevent coercion crashes and limit input lengths.
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
69 changes: 69 additions & 0 deletions tests/testthat/test-sentinel-validation.R
Original file line number Diff line number Diff line change
Expand Up @@ -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 handles interactive confirmCommonItems readline DoS inputs correctly", {
# Mock interactive() and readline() to simulate abnormally large integer input that used to coerce to NA
mockery::stub(aFIPC::autoFIPC, 'interactive', TRUE)
mockery::stub(aFIPC::autoFIPC, 'readline', mockery::mock('9999999999999999999', '9999999999999999999', '9999999999999999999'))

expect_error(
aFIPC::autoFIPC(
newformXData = data.frame(item1 = c(1,0,1)),
oldformYData = data.frame(item1 = c(1,1,0)),
newformCommonItemNames = c('item1'),
oldformCommonItemNames = c('item1'),
confirmCommonItems = NULL
),
"Too many invalid common item confirmation attempts"
)
})

test_that("autoFIPC handles interactive oldformBILOGprior readline DoS inputs correctly", {
mockery::stub(aFIPC::autoFIPC, 'interactive', TRUE)
mockery::stub(aFIPC::autoFIPC, 'readline', mockery::mock('1', '9999999999999999999', '9999999999999999999', '9999999999999999999'))

expect_error(
aFIPC::autoFIPC(
newformXData = data.frame(item1 = c(1,0,1)),
oldformYData = data.frame(item1 = c(1,1,0)),
newformCommonItemNames = c('item1'),
oldformCommonItemNames = c('item1'),
confirmCommonItems = NULL,
itemtype = '3PL',
oldformBILOGprior = NULL
),
"Too many invalid oldform BILOG prior attempts"
)
})

test_that("autoFIPC handles interactive newformBILOGprior readline DoS inputs correctly", {
mockery::stub(aFIPC::autoFIPC, 'interactive', TRUE)
mockery::stub(aFIPC::autoFIPC, 'readline', mockery::mock('1', '1', '9999999999999999999', '9999999999999999999', '9999999999999999999'))

# Stub out mirt::mirt but instead of failing with an error, return a proper S4 object Mock
mockClass <- setClass("MockClass", slots=c(OptimInfo="list"))

mock_mirt <- function(data, model, itemtype, ...) {
if (identical(data, data.frame(item1 = c(1,1,0)))) {
return(mockClass(OptimInfo=list(secondordertest=TRUE)))
}
stop('Should not reach here')
}

mockery::stub(aFIPC::autoFIPC, 'mirt::mirt', mock_mirt)

mockery::stub(aFIPC::autoFIPC, 'mirt::extract.mirt', function(...) data.frame(a1=1, d=0, g=0, u=1))
mockery::stub(aFIPC::autoFIPC, 'mirt::coef', function(...) list(item1=c(a1=1, d=0, g=0, u=1)))

expect_error(
aFIPC::autoFIPC(
newformXData = data.frame(item1 = c(1,0,1)),
oldformYData = data.frame(item1 = c(1,1,0)),
newformCommonItemNames = c('item1'),
oldformCommonItemNames = c('item1'),
confirmCommonItems = NULL,
itemtype = '3PL',
oldformBILOGprior = NULL,
newformBILOGprior = NULL
),
"Too many invalid newform BILOG prior attempts"
)
})
Loading