Skip to content
Open
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
1 change: 1 addition & 0 deletions .Rbuildignore
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,4 @@
^\.jules(/.*)?$
^\.trivyignore\.yaml$
^trivy\.yaml$
^\.semgrepignore$
6 changes: 6 additions & 0 deletions .jules/sentinel.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,9 @@
**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-05-18 - Insecure regex binding vulnerability in readline input
**취약점:** `aFIPC::autoFIPC()` λ‚΄λΆ€μ˜ μΈν„°λž™ν‹°λΈŒ `readline` μž…λ ₯ 검증에 μ•½ν•œ μ •κ·œμ‹ `^[0-9]+$` κ°€ μ‚¬μš©λ¨.
**ν•™μŠ΅:** `as.integer()`λŠ” `2^31 - 1` κΉŒμ§€λ§Œ μ²˜λ¦¬ν•  수 있으며, 이 λ²”μœ„λ₯Ό λ„˜λŠ” μž„μ˜μ˜ κΈ΄ μˆ«μžμ—΄μ΄ μž…λ ₯될 경우 쑰용히 `NA`λ₯Ό λ°˜ν™˜ν•¨. 이둜 인해 λ‹€μš΄μŠ€νŠΈλ¦Ό 둜직이 κΉ¨μ§€λŠ” 취약점이 λ°œμƒν•¨.
**λ°©μ§€:** μž…λ ₯을 μ •ν™•ν•˜κ²Œ μ œν•œν•΄μ•Ό 함. 1κ³Ό 2만 μ„ νƒλ°›λŠ” ν”„λ‘¬ν”„νŠΈλΌλ©΄, `^[12]$` 와 같이 μ—„κ²©ν•œ μ •κ·œμ‹μ„ μ‚¬μš©ν•˜μ—¬ μž…λ ₯을 필터링함.
2 changes: 1 addition & 1 deletion DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -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
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
36 changes: 36 additions & 0 deletions tests/testthat/test-sentinel-validation-fix.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
test_that("Sentinel: autoFIPC readline prompt uses strict regex", {
# In R CMD check, we have the exported functions directly available.

# Mock interactive to bypass immediate stop
mockery::stub(autoFIPC, "interactive", function() TRUE)

# Mock checkCorrect interactive input - test strict bound bypass vs valid (1)
# When giving invalid inputs ("3", "4", "abc"), autoFIPC prompts again up to 3 times
# We provide a valid "1" on the second attempt to see if it bypasses the invalid inputs
mockery::stub(autoFIPC, "readline", mockery::mock("3", "1", cycle = TRUE))

# Prevent estimating models loop
mockery::stub(autoFIPC, "mirt::mirt", function(data, ...) {
mod <- new("SingleGroupClass")
mod@OptimInfo$converged <- TRUE
mod@OptimInfo$secondordertest <- TRUE
mod@Data$data <- data
return(mod)
})

res <- tryCatch({
autoFIPC(
newformXData = data.frame(V1=1, V2=2),
oldformYData = data.frame(V1=1, V2=2),
newformCommonItemNames = c('V1'),
oldformCommonItemNames = c('V1'),
confirmCommonItems = NULL
)
},
error = function(e) e$message
)

# We should reach the mirt stub, which will throw error downstream or return cleanly
# The fact that it doesn't throw 'Too many invalid common item confirmation attempts' means it correctly rejected '3' and accepted '1'
expect_false(grepl("Too many invalid", res))
})
Loading