Skip to content
Closed
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
5 changes: 5 additions & 0 deletions .jules/sentinel.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,8 @@
**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-08-24 - Prevent Information Disclosure via Input Validation
**Vulnerability:** Unvalidated user inputs (`nested`, `adj`, `conf.level`) in exported functions `vuongtest()` and `icci()` caused raw R errors deep inside internal logic, leaking internal execution context when `NA`, multiple elements, or incorrect types were provided.
**Learning:** Unvalidated arguments bypass top-level `stop(..., call. = FALSE)` safeguards. Evaluating `NA` in `if` conditions triggers 'missing value where TRUE/FALSE needed'.
**Prevention:** Always strictly validate the type, length, and bounds of user inputs at the very beginning of exported functions, explicitly checking for `is.na()`, to fail securely with `stop(..., call. = FALSE)`.
5 changes: 5 additions & 0 deletions R/icci.R
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,11 @@
#' @export
icci <- function(object1, object2, conf.level=.95, ll1=llcont, ll2=llcont) {

## Security Fix: Validate inputs to prevent raw R errors from leaking internal execution context
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
8 changes: 8 additions & 0 deletions R/vuongtest.R
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,14 @@
#' @export
vuongtest <- function(object1, object2, nested=FALSE, adj="none", ll1=llcont, ll2=llcont, score1=NULL, score2=NULL, vc1=vcov, vc2=vcov) {

## Security Fix: Validate inputs to prevent raw R errors from leaking internal execution context
if(!is.logical(nested) || length(nested) != 1 || is.na(nested)) {
stop("nested must be a single TRUE or FALSE value", call. = FALSE)
}
if(!is.character(adj) || length(adj) != 1 || is.na(adj) || !(adj %in% c("none", "aic", "bic"))) {
stop("adj must be one of 'none', 'aic', or 'bic'", call. = FALSE)
}
Comment on lines +101 to +107

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: New user-facing errors not recorded in NEWS

The added validation raises new user-facing errors in icci() and vuongtest(), but NEWS is not updated, which the repository conventions request for user-visible changes.

Open in 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