Skip to content

πŸ›‘οΈ Sentinel: [MEDIUM] Fix integer coercion DoS vulnerability in readline #202

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
seonghobae wants to merge 4 commits into
base: master
Choose a base branch
from
Open

πŸ›‘οΈ Sentinel: [MEDIUM] Fix integer coercion DoS vulnerability in readline #202

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7ad3034
πŸ›‘οΈ Sentinel: [MEDIUM] Fix integer coercion DoS vulnerability in readl…
seonghobae Aug 2, 2026
dfeb9b3
πŸ›‘οΈ Sentinel: [MEDIUM] Fix integer coercion DoS vulnerability in readl…
seonghobae Aug 2, 2026
6cf1270
πŸ›‘οΈ Sentinel: [MEDIUM] Fix integer coercion DoS vulnerability in readl…
seonghobae Aug 2, 2026
f67f277
πŸ›‘οΈ Sentinel: [MEDIUM] Fix integer coercion DoS vulnerability in readl…
seonghobae Aug 2, 2026
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
1 change: 1 addition & 0 deletions .Rbuildignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,4 @@
^\.jules(/.*)?$
^\.trivyignore\.yaml$
^trivy\.yaml$
^\.semgrepignore$
5 changes: 5 additions & 0 deletions .jules/sentinel.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,8 @@
**Vulnerability:** Unvalidated inputs passed to `if()` statements can cause process crashes (`condition has length > 1`) or unexpected coercion vulnerabilities.
**Learning:** In R, optional boolean parameters that default to `NULL` should be validated using explicit runtime type validation (e.g., `if (!is.null(flag) && (!is.logical(flag) || length(flag) != 1 || is.na(flag)))`).
**Prevention:** Always implement explicit runtime type validation for optional boolean parameters.

## 2024-08-02 - Fix integer coercion DoS vulnerability
**Vulnerability:** Weak regex `^[0-9]+$` on interactive `readline()` user inputs allows huge numeric strings which coerce to `NA` when passed to `as.integer()`.
**Learning:** In R, evaluating `NA` inside an `if()` condition or returning `NA` from a function expected to return integers can break program logic or crash automation processes.
**Prevention:** Use strictly bounded exact-match regex like `^[12]$` when validating integer inputs intended to be explicitly mapped to fixed choices.
2 changes: 1 addition & 1 deletion DESCRIPTION
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ Description: Automates fixed item parameter linking for test linking under
the item response theory paradigm using mirt package estimates.
License: GPL-3 | file LICENSE
Imports: mirt, methods
Suggests: testthat (>= 3.0.0)
Suggests: testthat (>= 3.0.0), mockery
Encoding: UTF-8
Config/testthat/edition: 3
Config/roxygen2/version: 8.0.0
6 changes: 3 additions & 3 deletions R/aFIPC.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ autoFIPC <-
}
for (attempt in seq_len(3)) {
n <- readline(prompt = "Is it correct? (1: Yes 2: No) : ")
if (grepl("^[0-9]+$", n)) {
if (grepl("^[12]$", n)) {
return(as.integer(n))
}
}
Expand Down Expand Up @@ -171,7 +171,7 @@ autoFIPC <-
readline(
prompt = "Do you want to use default BILOG-MG priors for oldform Data? (1: Yes 2: No) : "
)
if (grepl("^[0-9]+$", n)) {
if (grepl("^[12]$", n)) {
return(as.integer(n))
}
}
Expand Down Expand Up @@ -390,7 +390,7 @@ autoFIPC <-
readline(
prompt = "Do you want to use default BILOG-MG priors for newform Data? (1: Yes 2: No) : "
)
if (grepl("^[0-9]+$", n)) {
if (grepl("^[12]$", n)) {
return(as.integer(n))
}
}
Expand Down
2 changes: 0 additions & 2 deletions test_dummy.R
View file
Open in desktop

This file was deleted.

3 changes: 0 additions & 3 deletions test_validation.R
View file
Open in desktop

This file was deleted.

43 changes: 43 additions & 0 deletions tests/testthat/test-sentinel-validation.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,46 @@ test_that("autoFIPC validates boolean flags for newformBILOGprior, oldformBILOGp
"Security Error: confirmCommonItems must be a single non-NA logical value or NULL"
)
})

test_that("autoFIPC integer validation works with strictly bounded regex", {
# Mock readline to return invalid inputs that should fail after 3 attempts
# override interactive() to TRUE to trigger the readline loop, then mock readline
with_mockery <- function() {
mockery::stub(aFIPC::autoFIPC, 'interactive', TRUE)
mockery::stub(aFIPC::autoFIPC, 'readline', mockery::mock('3', '0', '999999999'))
Comment on lines +43 to +44

@coderabbitai coderabbitai Bot Aug 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚑ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# μ €μž₯μ†Œ manifest에 mockery 선언이 μžˆλŠ”μ§€ ν™•μΈν•©λ‹ˆλ‹€.
files="$(fd -HI -t f '^(DESCRIPTION|renv\.lock|pak\.lock)$' . || true)"
test -n "$files" || { echo "manifestλ₯Ό 찾지 λͺ»ν–ˆμŠ΅λ‹ˆλ‹€." >&2; exit 2; }

if ! printf '%s\n' "$files" | xargs -r rg -n '\bmockery\b'; then
  echo "mockeryκ°€ manifest에 μ„ μ–Έλ˜μ§€ μ•Šμ•˜μŠ΅λ‹ˆλ‹€." >&2
  exit 1
fi

Repository: ContextualWisdomLab/aFIPC

Length of output: 2582

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- tracked manifests ---'
git ls-files | grep -E '(^|/)(DESCRIPTION|renv\.lock|pak\.lock|packrat/[^/]+)$' | head -80

printf '%s\n' '--- root DESCRIPTION ---'
if test -f DESCRIPTION; then
  cat -n DESCRIPTION
else
  echo 'DESCRIPTION not found'
fi

printf '%s\n' '--- test references and dependency guards ---'
rg -n -C 3 'mockery|skip_if_not_installed|testthat' tests/testthat/test-sentinel-validation.R DESCRIPTION .github 2>/dev/null || true

printf '%s\n' '--- package-managed mockery references ---'
rg -n --glob 'DESCRIPTION' --glob 'renv.lock' --glob 'pak.lock' --glob 'packrat.lock' --glob '!packrat/lib/**' '\bmockery\b' . 2>/dev/null || true

Repository: ContextualWisdomLab/aFIPC

Length of output: 6784

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- CI and package installation configuration ---'
git ls-files | grep -E '(^|/)(\.github/workflows/|appveyor|travis|Makefile|README|CONTRIBUTING)' | head -120
rg -n -C 3 'R CMD check|rcmdcheck|pak::|install|packrat|Suggests|mockery' \
  .github DESCRIPTION README* Makefile .travis.yml appveyor.yml 2>/dev/null || true

printf '%s\n' '--- packrat lock entries for relevant packages ---'
rg -n -C 2 '"(mockery|testthat|mirt)"|Package: (mockery|testthat|mirt)' \
  packrat/packrat.lock packrat 2>/dev/null | head -120 || true

printf '%s\n' '--- test file context ---'
cat -n tests/testthat/test-sentinel-validation.R | sed -n '1,95p'

Repository: ContextualWisdomLab/aFIPC

Length of output: 9661

🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path
import re

description = Path("DESCRIPTION").read_text(encoding="utf-8")
test_file = Path("tests/testthat/test-sentinel-validation.R").read_text(encoding="utf-8")
workflow = Path(".github/workflows/r.yml").read_text(encoding="utf-8")
lockfile = Path("packrat/packrat.lock").read_text(encoding="utf-8")

suggests = re.search(
    r"(?ms)^Suggests:\s*(.*?)(?=^[A-Za-z][A-Za-z0-9.-]*:|\Z)",
    description,
)
suggests_text = suggests.group(1) if suggests else ""
print("mockery_in_DESCRIPTION_Suggests:", bool(re.search(r"\bmockery\b", suggests_text)))
print("mockery_test_calls:", len(re.findall(r"\bmockery::(?:stub|mock)\s*\(", test_file)))
print("test_dependency_guard:", bool(re.search(r"skip_if_not_installed\s*\(\s*[\"']mockery", test_file)))
print("r_ci_uses_setup_r_dependencies:", "r-lib/actions/setup-r-dependencies@" in workflow)
print("mockery_in_packrat_lock:", bool(re.search(r"(?m)^Package:\s*mockery\s*$", lockfile)))
PY

Repository: ContextualWisdomLab/aFIPC

Length of output: 321

mockeryλ₯Ό Suggests에 μΆ”κ°€ν•˜μ‹­μ‹œμ˜€.

ν˜„μž¬ DESCRIPTIONκ³Ό packrat/packrat.lock에 mockeryκ°€ μ—†μŠ΅λ‹ˆλ‹€. ν…ŒμŠ€νŠΈλŠ” mockery::stub()와 mockery::mock()λ₯Ό 직접 ν˜ΈμΆœν•˜λ―€λ‘œ κΉ¨λ—ν•œ ν™˜κ²½μ˜ R CMD checkμ—μ„œ μ‹€νŒ¨ν•©λ‹ˆλ‹€. .github/workflows/r.yml의 μ˜μ‘΄μ„± 섀정이 μ„ μ–Έλœ Suggests νŒ¨ν‚€μ§€λ₯Ό μ„€μΉ˜ν•©λ‹ˆλ‹€. 선택적 μ˜μ‘΄μ„±μœΌλ‘œ μœ μ§€ν•˜λ €λ©΄ skip_if_not_installed("mockery")λ₯Ό μΆ”κ°€ν•˜μ‹­μ‹œμ˜€.

πŸ€– Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/testthat/test-sentinel-validation.R` around lines 43 - 44, Add mockery
to DESCRIPTION under Suggests and update packrat/packrat.lock accordingly so
test-sentinel-validation.R dependencies are declared; if retaining it as
optional, add skip_if_not_installed("mockery") before the
mockery::stub/mockery::mock calls.

expect_error(
aFIPC::autoFIPC(
newformXData = data.frame(A=1, B=2),
oldformYData = data.frame(A=1, B=2),
newformCommonItemNames = c('A', 'B'),
oldformCommonItemNames = c('A', 'B'),
confirmCommonItems = NULL
),
"Too many invalid common item confirmation attempts"
)
}
with_mockery()
})

test_that("autoFIPC integer validation accepts valid inputs", {
# Mock readline to return '1' (valid input)
with_mockery_valid <- function() {
mockery::stub(aFIPC::autoFIPC, 'interactive', TRUE)
mockery::stub(aFIPC::autoFIPC, 'readline', '1')
mockery::stub(aFIPC::autoFIPC, 'mirt::mirt', function(...) stop('forced failure'))
mockery::stub(aFIPC::autoFIPC, 'surveyFA', function(...) stop('forced failure'))

expect_error(
aFIPC::autoFIPC(
newformXData = data.frame(A=1, B=2),
oldformYData = data.frame(A=1, B=2),
newformCommonItemNames = c('A', 'B'),
oldformCommonItemNames = c('A', 'B'),
confirmCommonItems = NULL,
oldformBILOGprior = NULL,
newformBILOGprior = NULL
)
)
Comment on lines +67 to +77

@coderabbitai coderabbitai Bot Aug 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚑ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# 검증 이전 였λ₯˜μ™€ 검증 이후 였λ₯˜λ₯Ό ꡬ뢄할 수 μžˆλŠ”μ§€ ν™•μΈν•©λ‹ˆλ‹€.
rg -n -C 4 \
  'expect_error\(|Common item confirmation requires|Initial estimation of oldFormModel' \
  tests/testthat/test-sentinel-validation.R R/aFIPC.R

Repository: ContextualWisdomLab/aFIPC

Length of output: 4500

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- test block ---'
cat -n tests/testthat/test-sentinel-validation.R | sed -n '40,90p'

printf '%s\n' '--- autoFIPC control flow ---'
cat -n R/aFIPC.R | sed -n '100,250p'

printf '%s\n' '--- relevant test helpers and stubs ---'
rg -n -C 5 \
  'mirt::mirt|surveyFA|confirmCommonItems|oldformBILOGprior|newformBILOGprior|expect_error' \
  tests/testthat R

Repository: ContextualWisdomLab/aFIPC

Length of output: 50381

κΈ°λŒ€ν•œ 였λ₯˜λ₯Ό λͺ…μ‹œν•˜μ‹­μ‹œμ˜€.

interactive()λŠ” TRUE둜 μŠ€ν…λ˜κ³  readline()은 "1"을 λ°˜ν™˜ν•˜λ―€λ‘œ Line 136의 였λ₯˜λŠ” λ°œμƒν•˜μ§€ μ•ŠμŠ΅λ‹ˆλ‹€. κ·ΈλŸ¬λ‚˜ regexpκ°€ μ—†μ–΄μ„œ λ‹€λ₯Έ 였λ₯˜λ„ ν…ŒμŠ€νŠΈλ₯Ό ν†΅κ³Όμ‹œν‚΅λ‹ˆλ‹€. regexp = "Security Error: Initial estimation of oldFormModel completely failed"λ₯Ό μΆ”κ°€ν•˜μ‹­μ‹œμ˜€.

πŸ€– Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/testthat/test-sentinel-validation.R` around lines 67 - 77, Update the
autoFIPC error assertion in the sentinel validation test to require the specific
expected error message by adding the regexp pattern β€œSecurity Error: Initial
estimation of oldFormModel completely failed” to expect_error. Keep the existing
test setup unchanged.

}
with_mockery_valid()
})
Loading