Skip to content

T-328/T-323: guard Wagner Rcpp-boundary indices (tip_data values, addition_order)#261

Merged
ms609 merged 1 commit into
cpp-searchfrom
feature/t328-t323-wagner-rcpp-guards
Jul 4, 2026
Merged

T-328/T-323: guard Wagner Rcpp-boundary indices (tip_data values, addition_order)#261
ms609 merged 1 commit into
cpp-searchfrom
feature/t328-t323-wagner-rcpp-guards

Conversation

@ms609

@ms609 ms609 commented Jul 3, 2026

Copy link
Copy Markdown
Owner

Summary

  • Fixes T-328 (P2): simplify_patterns indexed token_states[tip_data - 1] with no range check, so an out-of-range tip_data value passed to the unexported Wagner entry points (ts_wagner_tree, ts_random_wagner_tree, ts_resample_search, ts_parallel_resample, ts_successive_approx, ts_simplify_diag) read out of bounds (0 → before the array; > n_tokens → past the end, likely a segfault).
  • Bundled with the sibling T-323 (P2): ts_wagner_tree converted addition_order 1-based → 0-based with no length/range/duplicate check — a short vector read past its end (reproduced segfault), and an out-of-range or duplicated value corrupted the tree via OOB heap writes.
  • Adds two shared boundary-validation helpers (validate_tip_data_values, validate_addition_order) in src/ts_rcpp.cpp, called from every affected entry point. ts_simplify_diag also gained its first weight/levels length guards (it had none) while touching that function.
  • Both gaps are internal-boundary-only: not reachable via the public AdditionTree()/MaximizeParsimony() API, which always builds tip_data/order from a validated phyDat — only reachable via TreeSearch::: with a hand-crafted matrix/vector.

Test plan

  • Rscript dev/build-fast.R — clean build
  • tests/testthat/test-ts-wagner.R — 57/57 pass locally (10 new tests covering tip_data value 0/negative/>n_tokens and addition_order wrong-length/out-of-range/zero/duplicate, each asserting a clean Rcpp::stop instead of a crash, across all 6 affected entry points)
  • Local covr check on every new/changed line in src/ts_rcpp.cpp — 100% covered
  • GHA ASan (gcc-ASAN) — triggers automatically on this PR (src/**, tests/testthat/**.R paths); local ASan is blocked on this machine, so this is the authoritative confirmation that the fixed OOB reads no longer fire

…y (T-328, T-323)

simplify_patterns() indexed token_states[tip_data-1] with no range check,
so an out-of-range tip_data value from a hand-crafted matrix passed to the
unexported ts_wagner_tree()/ts_random_wagner_tree()/ts_resample_search()/
ts_parallel_resample()/ts_successive_approx()/ts_simplify_diag() entry
points read out of bounds (0 -> before the array; > n_tokens -> past the
end, likely a segfault). Add a shared validate_tip_data_values() check at
the single Rcpp boundary and its four duplicated call sites (Rcpp::stop
on violation), and give ts_simplify_diag its first weight/levels length
guards while there.

Bundled with the sibling T-323 gap: ts_wagner_tree's addition_order was
converted 1-based -> 0-based with no length/range/duplicate check, so a
short vector read past its end (reproduced segfault) and an out-of-range
or duplicated value corrupted the tree via OOB heap writes. Add
validate_addition_order() mirroring the existing weight/levels/min_steps
guards.

Only reachable via TreeSearch:::, not the public API (AdditionTree()/
MaximizeParsimony() always build valid inputs from a checked phyDat).
@ms609 ms609 force-pushed the feature/t328-t323-wagner-rcpp-guards branch from 4f8c5bd to 87f0a16 Compare July 4, 2026 05:09
@ms609 ms609 merged commit 61f7147 into cpp-search Jul 4, 2026
7 of 9 checks passed
@ms609 ms609 deleted the feature/t328-t323-wagner-rcpp-guards branch July 4, 2026 11:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant