Skip to content
4 changes: 4 additions & 0 deletions .jules/sentinel.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
4 changes: 4 additions & 0 deletions R/icci.R
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions R/vuongtest.R
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Comment on lines +104 to +106

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: adj now errors on previously-tolerated inputs

The old code compared adj only against "aic"/"bic", so any other value silently acted as "none". The strict check now errors on such inputs, a behavior change for callers that passed unexpected adj values.

Devin Review

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


## check objects, issue warnings/errors, get classes/calls
obinfo <- check.obj(object1, object2)
callA <- obinfo$callA; classA <- obinfo$classA
Expand Down
Loading