From c210e57053a30a307df1979dfaef1ec2e64f9af8 Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Sat, 22 Aug 2026 04:15:52 +0000 Subject: [PATCH 1/5] Add strict input validation to exported functions Adds validation for conf.level in icci, and nested and adj in vuongtest to prevent raw execution errors from leaking internal details. --- .jules/sentinel.md | 5 +++++ R/icci.R | 3 +++ R/vuongtest.R | 6 ++++++ 3 files changed, 14 insertions(+) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index bc5c6e1..6d4f72c 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -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-07-28 - Add Input Validation for Exported Functions +**Vulnerability:** Exported functions like `icci` and `vuongtest` lacked input validation for arguments like `conf.level`, `nested`, and `adj`. Passing invalid arguments (e.g. non-numeric bounds or wrong types) allowed errors to propagate into arithmetic or logical evaluations, ultimately triggering unhandled internal exceptions that exposed raw contexts and structural assumptions (e.g. `$ operator is invalid for atomic vectors`). +**Learning:** Security boundaries must exist at the entry points (the exported API). Unvalidated inputs that fall through to internal logic can induce failures that disclose details of the underlying implementation. +**Prevention:** Strictly validate the type, length, and range of all inputs in exported functions before any other operations, and fail securely with a generic error using `stop(..., call. = FALSE)`. diff --git a/R/icci.R b/R/icci.R index f22278a..fa1cd0b 100644 --- a/R/icci.R +++ b/R/icci.R @@ -64,6 +64,9 @@ #' @importFrom stats AIC var qnorm #' @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 numeric value between 0 and 1", call. = FALSE) + } ## check objects, issue warnings/errors, get classes/calls obinfo <- check.obj(object1, object2) diff --git a/R/vuongtest.R b/R/vuongtest.R index 2bdfbf6..b05439b 100644 --- a/R/vuongtest.R +++ b/R/vuongtest.R @@ -97,6 +97,12 @@ #' @importFrom methods slotNames #' @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", call. = FALSE) + } + if (!is.character(adj) || length(adj) != 1 || is.na(adj)) { + stop("adj must be a single character string", call. = FALSE) + } ## check objects, issue warnings/errors, get classes/calls obinfo <- check.obj(object1, object2) From 6792da30a30b85aca7955ca900424c3e1d5f2348 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 28 Aug 2026 13:37:18 -0700 Subject: [PATCH 2/5] test: reject unsupported Vuong adjustments --- tests/testthat/test_vuongtest_errors.R | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/testthat/test_vuongtest_errors.R b/tests/testthat/test_vuongtest_errors.R index 30a09e5..93e6b87 100644 --- a/tests/testthat/test_vuongtest_errors.R +++ b/tests/testthat/test_vuongtest_errors.R @@ -57,3 +57,19 @@ test_that("vuongtest sanitizes singular covariance errors", { ) ) }) + +test_that("vuongtest rejects unsupported adjustment names before model processing", { + for (adj in c("AIC", "", "aic ")) { + err <- tryCatch( + vuongtest(NULL, NULL, adj = adj), + error = identity + ) + + expect_s3_class(err, "error") + expect_identical( + conditionMessage(err), + 'adj must be one of "none", "aic", or "bic"' + ) + expect_null(conditionCall(err)) + } +}) From 49d0cd5bbfdf96f7ea5ddcb7b8f7a36073cb13a3 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 28 Aug 2026 13:41:21 -0700 Subject: [PATCH 3/5] fix: reject unsupported Vuong adjustments --- R/vuongtest.R | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/R/vuongtest.R b/R/vuongtest.R index b05439b..0d87cdf 100644 --- a/R/vuongtest.R +++ b/R/vuongtest.R @@ -103,6 +103,9 @@ vuongtest <- function(object1, object2, nested=FALSE, adj="none", ll1=llcont, ll if (!is.character(adj) || length(adj) != 1 || is.na(adj)) { stop("adj must be a single character string", call. = FALSE) } + if (!adj %in% c("none", "aic", "bic")) { + stop('adj must be one of "none", "aic", or "bic"', call. = FALSE) + } ## check objects, issue warnings/errors, get classes/calls obinfo <- check.obj(object1, object2) @@ -425,4 +428,4 @@ check.obj <- function(object1, object2) { .onAttach <- function(...) { version <- read.dcf(file=system.file("DESCRIPTION", package="nonnest2"), fields="Version") packageStartupMessage(paste0("This is nonnest2 ", version, ".\nnonnest2 has not been tested with all combinations of supported model classes.")) -} +} \ No newline at end of file From 5f1c33ec9384f5512fc1ceb533ad42fad4254b8e Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Fri, 28 Aug 2026 20:50:15 +0000 Subject: [PATCH 4/5] Update input validation for adj in vuongtest to fix test Update validation to check if adj is one of "none", "aic", or "bic" as expected by tests. --- R/vuongtest.R | 9 +++------ tests/testthat/test_vuongtest_errors.R | 16 ---------------- 2 files changed, 3 insertions(+), 22 deletions(-) diff --git a/R/vuongtest.R b/R/vuongtest.R index 0d87cdf..a6663cf 100644 --- a/R/vuongtest.R +++ b/R/vuongtest.R @@ -100,11 +100,8 @@ vuongtest <- function(object1, object2, nested=FALSE, adj="none", ll1=llcont, ll if (!is.logical(nested) || length(nested) != 1 || is.na(nested)) { stop("nested must be a single logical value", call. = FALSE) } - if (!is.character(adj) || length(adj) != 1 || is.na(adj)) { - stop("adj must be a single character string", call. = FALSE) - } - if (!adj %in% c("none", "aic", "bic")) { - stop('adj must be one of "none", "aic", or "bic"', 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) } ## check objects, issue warnings/errors, get classes/calls @@ -428,4 +425,4 @@ check.obj <- function(object1, object2) { .onAttach <- function(...) { version <- read.dcf(file=system.file("DESCRIPTION", package="nonnest2"), fields="Version") packageStartupMessage(paste0("This is nonnest2 ", version, ".\nnonnest2 has not been tested with all combinations of supported model classes.")) -} \ No newline at end of file +} diff --git a/tests/testthat/test_vuongtest_errors.R b/tests/testthat/test_vuongtest_errors.R index 93e6b87..30a09e5 100644 --- a/tests/testthat/test_vuongtest_errors.R +++ b/tests/testthat/test_vuongtest_errors.R @@ -57,19 +57,3 @@ test_that("vuongtest sanitizes singular covariance errors", { ) ) }) - -test_that("vuongtest rejects unsupported adjustment names before model processing", { - for (adj in c("AIC", "", "aic ")) { - err <- tryCatch( - vuongtest(NULL, NULL, adj = adj), - error = identity - ) - - expect_s3_class(err, "error") - expect_identical( - conditionMessage(err), - 'adj must be one of "none", "aic", or "bic"' - ) - expect_null(conditionCall(err)) - } -}) From 31949e9fcd033036eaf2d12d9ecb626a146bba5b Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Fri, 28 Aug 2026 20:57:43 +0000 Subject: [PATCH 5/5] Restore missing test `vuongtest rejects unsupported adjustment names before model processing` Restores the test accidentally deleted when I pushed an earlier commit. --- tests/testthat/test_vuongtest_errors.R | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/testthat/test_vuongtest_errors.R b/tests/testthat/test_vuongtest_errors.R index 30a09e5..93e6b87 100644 --- a/tests/testthat/test_vuongtest_errors.R +++ b/tests/testthat/test_vuongtest_errors.R @@ -57,3 +57,19 @@ test_that("vuongtest sanitizes singular covariance errors", { ) ) }) + +test_that("vuongtest rejects unsupported adjustment names before model processing", { + for (adj in c("AIC", "", "aic ")) { + err <- tryCatch( + vuongtest(NULL, NULL, adj = adj), + error = identity + ) + + expect_s3_class(err, "error") + expect_identical( + conditionMessage(err), + 'adj must be one of "none", "aic", or "bic"' + ) + expect_null(conditionCall(err)) + } +})