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-10-18 - Prevent Information Disclosure via Unvalidated Arguments
**Vulnerability:** Missing type, length, and bound validations on arguments passed to exported functions (`vuongtest()` and `icci()`) allowed malformed inputs (e.g., `NA` or invalid types) to reach internal logical conditions (`if(nested)`), leaking internal execution contexts via raw R errors.
**Learning:** Unvalidated arguments in exported functions can bypass top-level `stop(..., call. = FALSE)` safeguards and trigger raw R errors deep inside internal logic.
**Prevention:** Always strictly validate the type, length, and bounds of user inputs at the very beginning of exported functions to fail securely with `call. = FALSE`.
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("The 'conf.level' argument must be a single numeric value strictly between 0 and 1.", call. = FALSE)
Comment on lines +68 to +69

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

πŸ” Validation change missing from changelog

The exported functions now reject previously accepted inputs, but NEWS omits this user-visible change. Repository guidance requires recording it.

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
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("The 'nested' argument 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("The 'adj' argument must be a single character string: 'none', 'aic', or 'bic'.", call. = FALSE)
Comment on lines +101 to +105

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

πŸ” Validation paths lack regression tests

No tests cover malformed conf.level, nested, or adj values and their sanitized errors. Future changes can silently weaken these exported API boundaries.

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