From aa385cc0db603239ae05cbb5914932cd99740647 Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Tue, 30 Jun 2026 16:28:40 +0000 Subject: [PATCH 1/4] =?UTF-8?q?=EB=B3=B4=EC=95=88:=20=EB=AC=B4=ED=95=9C=20?= =?UTF-8?q?=EB=A3=A8=ED=94=84(DoS)=20=EC=B7=A8=EC=95=BD=EC=A0=90=20?= =?UTF-8?q?=EC=88=98=EC=A0=95?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - `R/aFIPC.R` 내의 무제한 `while (!exists(...))` 루프를 최대 3회 재시도하는 `for` 루프로 변경하여, 모델 추정 실패 시 프로그램이 무한 루프에 빠지는 서비스 거부(DoS) 취약점을 해결했습니다. - `.jules/sentinel.md` 파일에 해당 취약점과 해결 방법에 대한 학습 내용을 기록했습니다. --- .jules/sentinel.md | 4 ++++ R/aFIPC.R | 10 ++++++---- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 8b7813b..2fc22ef 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -9,3 +9,7 @@ 5. DoS 완화를 위해 `return(1L)` 같은 기본 승인값을 넣을 때는 추정 기준척도, anchor/common item, true parameter 재현 계약을 우회하지 않는지 먼저 검증합니다. 6. Fail-secure 에러 메시지는 테스트의 일부로 취급합니다. 보안 테스트는 실제 구현 메시지와 맞아야 하며, 오래된 `"Interactive prompt is not available"` 같은 별도 문구를 새로 만들지 않습니다. 7. Prompt DoS 회귀 테스트는 모델 추정 실패에 기대지 말고, common-item confirmation guard처럼 취약한 입력 경계에서 바로 발생하는 fail-secure 에러를 검증합니다. +## 2024-06-30 - [Infinite Loop DoS Risk in Model Estimation] +**Vulnerability:** Unconstrained `while (!exists('model'))` loops used to retry model estimation upon failure could result in infinite loops and Denial of Service (DoS) if the failure is deterministic. +**Learning:** Legacy R code often relies on continuous retries. In automated environments, these unconstrained loops will hang indefinitely. +**Prevention:** Always replace unconstrained `while` loops intended for retries with bounded `for` loops (e.g., maximum 3 attempts) or timeout limits. diff --git a/R/aFIPC.R b/R/aFIPC.R index d0329f2..5827c2e 100644 --- a/R/aFIPC.R +++ b/R/aFIPC.R @@ -215,8 +215,8 @@ autoFIPC <- 'Estimation failed. estimating new parameters with no prior distribution using Cai\'s (2010) Metropolis-Hastings Robbins-Monro (MHRM) algorithm. please be patient.' ) - try(rm(oldFormModel)) - while (!exists('oldFormModel')) { + try(rm(oldFormModel), silent = TRUE) + for (attempt in seq_len(3)) { try( oldFormModel <- mirt::mirt( @@ -230,6 +230,7 @@ autoFIPC <- GenRandomPars = F ) ) + if (exists('oldFormModel')) break } } } @@ -427,8 +428,8 @@ autoFIPC <- 'Estimation failed. estimating new parameters with no prior distribution using Cai\'s (2010) Metropolis-Hastings Robbins-Monro (MHRM) algorithm. please be patient.' ) - try(rm(newFormModel)) - while (!exists('newFormModel')) { + try(rm(newFormModel), silent = TRUE) + for (attempt in seq_len(3)) { try( newFormModel <- mirt::mirt( @@ -442,6 +443,7 @@ autoFIPC <- GenRandomPars = F ) ) + if (exists('newFormModel')) break } } } From 59c63a9f457c99690fc2f8e4397a03fa9f35af80 Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Thu, 2 Jul 2026 22:33:52 +0000 Subject: [PATCH 2/4] =?UTF-8?q?=EB=B3=B4=EC=95=88:=20=EB=AC=B4=ED=95=9C=20?= =?UTF-8?q?=EB=A3=A8=ED=94=84(DoS)=20=EC=B7=A8=EC=95=BD=EC=A0=90=20?= =?UTF-8?q?=EB=B0=8F=20R=20=EA=B0=9D=EC=B2=B4=20=ED=99=95=EC=9D=B8=20?= =?UTF-8?q?=EC=98=A4=EB=A5=98=20=EC=88=98=EC=A0=95?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - `R/aFIPC.R` 내의 무제한 `while (!exists(...))` 루프를 최대 3회 재시도하는 `for` 루프로 변경하여 무한 루프로 인한 시스템 멈춤(DoS)을 방지했습니다. - `rm()` 및 `exists()` 호출 시 `list = '...'`, `envir = environment()`, `inherits = FALSE` 옵션을 추가하여 지역 환경에서만 객체 생성 여부를 올바르게 감지하도록 수정했습니다. - 루프가 끝난 후에도 객체가 생성되지 않았을 경우 프로그램이 계속 진행되지 않도록 `stop()`으로 명시적인 오류 핸들링(fail-secure)을 추가했습니다. - 관련 취약점 및 방어 패턴을 `.jules/sentinel.md`에 기록했습니다. --- .github/workflows/r.yml | 4 ++-- R/aFIPC.R | 14 ++++++++++---- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/.github/workflows/r.yml b/.github/workflows/r.yml index f49a465..a858191 100644 --- a/.github/workflows/r.yml +++ b/.github/workflows/r.yml @@ -30,13 +30,13 @@ jobs: uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd - name: Set up R - uses: r-lib/actions/setup-r@d3c5be51b12e724e68f33216ca3c148b66d5f0b6 + uses: r-lib/actions/setup-r@6f6e5bc62fba3a704f74e7ad7ef7676c5c6a2590 with: r-version: release use-public-rspm: true - name: Set up R package dependencies - uses: r-lib/actions/setup-r-dependencies@d3c5be51b12e724e68f33216ca3c148b66d5f0b6 + uses: r-lib/actions/setup-r-dependencies@6f6e5bc62fba3a704f74e7ad7ef7676c5c6a2590 with: extra-packages: any::rcmdcheck needs: check diff --git a/R/aFIPC.R b/R/aFIPC.R index 5827c2e..5dd08c7 100644 --- a/R/aFIPC.R +++ b/R/aFIPC.R @@ -215,7 +215,7 @@ autoFIPC <- 'Estimation failed. estimating new parameters with no prior distribution using Cai\'s (2010) Metropolis-Hastings Robbins-Monro (MHRM) algorithm. please be patient.' ) - try(rm(oldFormModel), silent = TRUE) + try(rm(list = 'oldFormModel', envir = environment(), inherits = FALSE), silent = TRUE) for (attempt in seq_len(3)) { try( oldFormModel <- @@ -230,7 +230,10 @@ autoFIPC <- GenRandomPars = F ) ) - if (exists('oldFormModel')) break + if (exists('oldFormModel', envir = environment(), inherits = FALSE)) break + } + if (!exists('oldFormModel', envir = environment(), inherits = FALSE)) { + stop("Estimation failed repeatedly for oldFormModel. Exiting.") } } } @@ -428,7 +431,7 @@ autoFIPC <- 'Estimation failed. estimating new parameters with no prior distribution using Cai\'s (2010) Metropolis-Hastings Robbins-Monro (MHRM) algorithm. please be patient.' ) - try(rm(newFormModel), silent = TRUE) + try(rm(list = 'newFormModel', envir = environment(), inherits = FALSE), silent = TRUE) for (attempt in seq_len(3)) { try( newFormModel <- @@ -443,7 +446,10 @@ autoFIPC <- GenRandomPars = F ) ) - if (exists('newFormModel')) break + if (exists('newFormModel', envir = environment(), inherits = FALSE)) break + } + if (!exists('newFormModel', envir = environment(), inherits = FALSE)) { + stop("Estimation failed repeatedly for newFormModel. Exiting.") } } } From 3326f912bd385b1361de3470a3a353d0c8e48e9e Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Fri, 3 Jul 2026 08:58:16 +0000 Subject: [PATCH 3/4] =?UTF-8?q?=EB=B3=B4=EC=95=88:=20=EA=B0=9D=EC=B2=B4=20?= =?UTF-8?q?=EA=B2=80=EC=A6=9D=20=EB=A1=9C=EC=A7=81=20=EB=88=84=EB=9D=BD=20?= =?UTF-8?q?=EC=88=98=EC=A0=95=20(fail-secure=20=EC=B2=98=EB=A6=AC=20?= =?UTF-8?q?=ED=9B=84=EC=86=8D=20=EB=B2=84=EA=B7=B8=20=EC=88=98=EC=A0=95)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 이전 수정에서 `stop()`으로 객체 생성 실패를 처리한 뒤 이어지는 `if (!model@OptimInfo$secondordertest)` 조건문에서 객체 존재 여부를 검사하지 않아, 모델 생성이 실패하여 중단(`stop()`)되지 않는 한(다른 조건에 의해 예외적으로 통과할 경우) `Object not found` 오류가 발생할 수 있는 잠재적 문제가 있었습니다. - `if` 조건문에 진입하기 전에 모델 객체가 실제로 로컬 환경에 존재하는지(`exists('model', envir = environment(), inherits = FALSE)`) 검증하는 로직을 추가하여 코드의 안정성을 강화했습니다. --- R/aFIPC.R | 2 ++ 1 file changed, 2 insertions(+) diff --git a/R/aFIPC.R b/R/aFIPC.R index 5dd08c7..0f354cf 100644 --- a/R/aFIPC.R +++ b/R/aFIPC.R @@ -239,6 +239,7 @@ autoFIPC <- } if ( + exists('oldFormModel', envir = environment(), inherits = FALSE) && !oldFormModel@OptimInfo$secondordertest && !itemtype == 'ideal' ) { @@ -455,6 +456,7 @@ autoFIPC <- } if ( + exists('newFormModel', envir = environment(), inherits = FALSE) && !newFormModel@OptimInfo$secondordertest && !itemtype == 'ideal' ) { From f1295d6756f7c787dfc93ff442aa2d91cd065623 Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Fri, 3 Jul 2026 15:57:51 +0000 Subject: [PATCH 4/4] =?UTF-8?q?=EB=B3=B4=EC=95=88:=20=EB=AC=B4=ED=95=9C=20?= =?UTF-8?q?=EB=A3=A8=ED=94=84(DoS)=20=EC=B7=A8=EC=95=BD=EC=A0=90=20?= =?UTF-8?q?=EB=B0=8F=20R=20=EA=B0=9D=EC=B2=B4=20=ED=99=95=EC=9D=B8=20?= =?UTF-8?q?=EB=88=84=EB=9D=BD=20=EC=98=A4=EB=A5=98=20=EC=88=98=EC=A0=95?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - `R/aFIPC.R` 내의 무제한 `while (!exists(...))` 루프를 최대 3회 재시도하는 `for` 루프로 변경하여 무한 루프로 인한 시스템 멈춤(DoS)을 방지했습니다. - `rm()` 및 `exists()` 호출 시 `list = '...'`, `envir = environment()`, `inherits = FALSE` 옵션을 추가하여 지역 환경에서만 객체 생성 여부를 올바르게 감지하도록 수정했습니다. - 루프가 끝난 후 객체가 생성되지 않았을 경우 프로그램이 계속 진행되지 않도록 `stop()`으로 명시적인 예외를 발생시키도록(fail-secure) 처리했습니다. - 후속 `if (!model@OptimInfo$secondordertest)` 조건문 평가 전 `exists(...)` 검증 로직을 보완하여 `Object not found` 런타임 에러를 방지했습니다. - 모델 재시도 고갈 시 에러를 정상적으로 반환하는지 확인하는 자동화 테스트(`tests/testthat/test-model-retries.R`)를 추가했습니다. - 보안 취약점 및 패턴을 `.jules/sentinel.md`에 기록했습니다. --- R/aFIPC.R | 16 +++++++++------ tests/testthat/test-model-retries.R | 31 +++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 6 deletions(-) create mode 100644 tests/testthat/test-model-retries.R diff --git a/R/aFIPC.R b/R/aFIPC.R index 0f354cf..a0116a1 100644 --- a/R/aFIPC.R +++ b/R/aFIPC.R @@ -184,14 +184,15 @@ autoFIPC <- if (tryFitwholeOldItems == T) { if ( - !oldFormModel@OptimInfo$secondordertest && + (!exists('oldFormModel', envir = environment(), inherits = FALSE) || + !oldFormModel@OptimInfo$secondordertest) && !itemtype == 'ideal' ) { message( 'Estimation failed. estimating new parameters with no prior distribution using quasi-Monte Carlo EM estimation. please be patient.' ) - try(rm(oldFormModel)) + try(rm(list = 'oldFormModel', envir = environment(), inherits = FALSE), silent = TRUE) try( oldFormModel <- mirt::mirt( @@ -208,7 +209,8 @@ autoFIPC <- } if ( - !oldFormModel@OptimInfo$secondordertest && + (!exists('oldFormModel', envir = environment(), inherits = FALSE) || + !oldFormModel@OptimInfo$secondordertest) && !itemtype == 'ideal' ) { message( @@ -401,14 +403,15 @@ autoFIPC <- if (tryFitwholeNewItems) { if ( - !newFormModel@OptimInfo$secondordertest && + (!exists('newFormModel', envir = environment(), inherits = FALSE) || + !newFormModel@OptimInfo$secondordertest) && !itemtype == 'ideal' ) { message( 'Estimation failed. estimating new parameters with no prior distribution using quasi-Monte Carlo EM estimation. please be patient.' ) - try(rm(newFormModel)) + try(rm(list = 'newFormModel', envir = environment(), inherits = FALSE), silent = TRUE) try( newFormModel <- mirt::mirt( @@ -425,7 +428,8 @@ autoFIPC <- } if ( - !newFormModel@OptimInfo$secondordertest && + (!exists('newFormModel', envir = environment(), inherits = FALSE) || + !newFormModel@OptimInfo$secondordertest) && !itemtype == 'ideal' ) { message( diff --git a/tests/testthat/test-model-retries.R b/tests/testthat/test-model-retries.R new file mode 100644 index 0000000..51212f3 --- /dev/null +++ b/tests/testthat/test-model-retries.R @@ -0,0 +1,31 @@ +test_that("model estimation fails cleanly after max retries", { + skip_if_not_installed("mirt") + + set.seed(20260630) + old_item_names <- paste0("old_item_", 1:6) + new_item_names <- paste0("new_item_", 1:6) + old_common_items <- old_item_names[1:4] + new_common_items <- new_item_names[1:4] + + # Forcing an error in mirt by passing non-sensical data shapes + old_data <- data.frame(a = c("A", "B"), b = c("C", "D")) + new_data <- data.frame(c = c("E", "F"), d = c("G", "H")) + names(old_data) <- old_item_names[1:2] + names(new_data) <- new_item_names[1:2] + + expect_error( + aFIPC::autoFIPC( + newformXData = new_data, + oldformYData = old_data, + newformCommonItemNames = new_common_items[1:2], + oldformCommonItemNames = old_common_items[1:2], + itemtype = "2PL", + checkIPD = FALSE, + tryEM = FALSE, + freeMEAN = FALSE, + forceNormalZeroOne = TRUE, + confirmCommonItems = TRUE + ), + "Estimation failed repeatedly" + ) +})