diff --git a/.jules/sentinel.md b/.jules/sentinel.md index bc5c6e1..c6876c0 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -12,3 +12,7 @@ **Vulnerability:** Raw `stop()` and `warning()` calls without `call. = FALSE` in `llcont.R` and `vuongtest.R` exposed execution stack/call details when raised. **Learning:** While some instances of `stop()` inside `tryCatch()` were previously fixed to hide the call stack, other standalone exceptions and warnings still leaked call context. Security must be consistently applied across the entire codebase. **Prevention:** Always set `call. = FALSE` when using `stop()` or `warning()` to enforce a secure-by-default boundary and prevent internal execution paths from being disclosed to the end user. +## 2024-10-31 - [Medium] Add input validation to exported functions to fail securely +**Vulnerability:** Missing strict type, length, and bounds checking on user-supplied arguments (`adj` in `vuongtest` and `conf.level` in `icci`) allowed unvalidated inputs to propagate into internal R evaluation, potentially causing unexpected raw execution errors and exposing internal context (stack traces). +**Learning:** R's default lazy evaluation and dynamic typing mean that unvalidated user inputs might evaluate deep inside package logic, bypassing intended top-level safeguards and leaking internal execution context upon failure. +**Prevention:** Always strictly validate the type, length, and bounds of user inputs at the very beginning of exported functions and fail securely using `stop(..., call. = FALSE)`. Ensure `is.na()` is explicitly checked before relational operators to avoid fatal 'missing value where TRUE/FALSE needed' errors. diff --git a/R/icci.R b/R/icci.R index f22278a..7aea703 100644 --- a/R/icci.R +++ b/R/icci.R @@ -65,6 +65,10 @@ #' @export icci <- function(object1, object2, conf.level=.95, ll1=llcont, ll2=llcont) { + if (!is.numeric(conf.level) || length(conf.level) != 1 || is.na(conf.level) || conf.level <= 0 || conf.level >= 1) { + stop("'conf.level' must be a single numeric value between 0 and 1", call. = FALSE) + } + ## check objects, issue warnings/errors, get classes/calls obinfo <- check.obj(object1, object2) callA <- obinfo$callA; classA <- obinfo$classA diff --git a/R/vuongtest.R b/R/vuongtest.R index 2bdfbf6..d2cd748 100644 --- a/R/vuongtest.R +++ b/R/vuongtest.R @@ -98,6 +98,13 @@ #' @export vuongtest <- function(object1, object2, nested=FALSE, adj="none", ll1=llcont, ll2=llcont, score1=NULL, score2=NULL, vc1=vcov, vc2=vcov) { + if (!is.logical(nested) || length(nested) != 1 || is.na(nested)) { + stop("'nested' must be a single logical value (TRUE or FALSE)", call. = FALSE) + } + if (!is.character(adj) || length(adj) != 1 || is.na(adj) || !(adj %in% c("none", "aic", "bic"))) { + stop("'adj' must be a single string matching 'none', 'aic', or 'bic'", call. = FALSE) + } + ## check objects, issue warnings/errors, get classes/calls obinfo <- check.obj(object1, object2) callA <- obinfo$callA; classA <- obinfo$classA