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
3 changes: 3 additions & 0 deletions .Rbuildignore
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,6 @@
^\.jules(/.*)?$
^\.trivyignore\.yaml$
^trivy\.yaml$
^test_dummy\.R$
^test_validation\.R$
^\.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-26 - μ •μˆ˜ μ˜€λ²„ν”Œλ‘œμš° κ°•μ œ λ³€ν™˜ 취약점 μˆ˜μ •
**Vulnerability:** λŒ€ν™”ν˜• `readline()` μž…λ ₯μ—μ„œ 숫자 μ˜΅μ…˜μ„ 검증할 λ•Œ μ œν•œ μ—†λŠ” 숫자 클래슀(예: `^[0-9]+$`)λ₯Ό μ‚¬μš©ν•˜λ©΄, λ„ˆλ¬΄ 큰 μˆ«μžκ°€ μ •κ·œν‘œν˜„μ‹ 검사λ₯Ό ν†΅κ³Όν•œ ν›„ `as.integer()`μ—μ„œ `NA`둜 ν‰κ°€λ˜μ–΄ 후속 ν”„λ‘œμ„ΈμŠ€ μΆ©λŒμ„ μœ λ°œν•  수 μžˆμŠ΅λ‹ˆλ‹€.
**Learning:** R μŠ€ν¬λ¦½νŠΈμ—μ„œ λŒ€ν™”ν˜• `readline()` 숫자 μž…λ ₯을 검증할 λ•Œ, μ œν•œ μ—†λŠ” 숫자 ν΄λž˜μŠ€λ³΄λ‹€ μ •ν™•νžˆ μ˜ˆμƒλ˜λŠ” κ°’(예: `^[12]$`)κ³Ό μ—„κ²©ν•˜κ²Œ μΌμΉ˜μ‹œμΌœμ•Ό ν•©λ‹ˆλ‹€.
**Prevention:** μ •μˆ˜ μ˜€λ²„ν”Œλ‘œμš° κ°•μ œ λ³€ν™˜ 취약점을 λ°©μ§€ν•˜κΈ° μœ„ν•΄ λŒ€ν™”ν˜• μ„Έμ…˜μ—μ„œ μ˜ˆμƒλ˜λŠ” 숫자 μž…λ ₯ μ˜΅μ…˜μ— λŒ€ν•΄ 항상 μ •λ°€ν•œ μ •κ·œν‘œν˜„μ‹ 맀칭을 μ‚¬μš©ν•΄μ•Ό ν•©λ‹ˆλ‹€.
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)) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

πŸ“ Info: Tightened regex also rejects other digits at BILOG prompts

At R/aFIPC.R:174 and R/aFIPC.R:393, inputs other than 1/2 previously returned the number and were treated as No via the else branch. They now exhaust the three attempts and error. This changes behavior for out-of-range interactive input, not just overflow values.

Open in Devin Review

Was this helpful? React with πŸ‘ or πŸ‘Ž to provide feedback.

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
61 changes: 61 additions & 0 deletions tests/testthat/test-sentinel-validation.R
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,64 @@ 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 overflow coercion vulnerability in readline inputs is handled safely", {
# We test the interactive prompt paths by mocking readline to simulate bad inputs.

# For confirmCommonItems prompt
mock_readline_confirm <- mockery::mock("3", "9999999999", "invalid", cycle = TRUE)
mockery::stub(aFIPC::autoFIPC, 'readline', mock_readline_confirm)
mockery::stub(aFIPC::autoFIPC, 'interactive', function() TRUE)

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"
)
})

test_that("autoFIPC input validation for required arguments", {
expect_error(
aFIPC::autoFIPC(
newformXData = 1, # Invalid type
oldformYData = data.frame(A=2),
newformCommonItemNames = c('A'),
oldformCommonItemNames = c('A')
),
"Security Error: newformXData must be a data.frame, matrix, or a valid fitted mirt model"
)

expect_error(
aFIPC::autoFIPC(
newformXData = data.frame(A=1),
oldformYData = 2, # Invalid type
newformCommonItemNames = c('A'),
oldformCommonItemNames = c('A')
),
"Security Error: oldformYData must be a data.frame, matrix, or a valid fitted mirt model"
)

expect_error(
aFIPC::autoFIPC(
newformXData = data.frame(A=1),
oldformYData = data.frame(A=2),
newformCommonItemNames = 1, # Invalid type
oldformCommonItemNames = c('A')
),
"Security Error: newformCommonItemNames must be a character vector"
)

expect_error(
aFIPC::autoFIPC(
newformXData = data.frame(A=1),
oldformYData = data.frame(A=2),
newformCommonItemNames = c('A'),
oldformCommonItemNames = 2 # Invalid type
),
"Security Error: oldformCommonItemNames must be a character vector"
)
})
Loading