fix: out-of-bounds read in NumericUnkMaker::checkPeriod (closes #157)#158
Merged
eiennohito merged 2 commits intomasterfrom Apr 17, 2026
Merged
fix: out-of-bounds read in NumericUnkMaker::checkPeriod (closes #157)#158eiennohito merged 2 commits intomasterfrom
eiennohito merged 2 commits intomasterfrom
Conversation
…digit+period `checkPeriod` bounded the lookahead with `pos + 1 < codepoints.size()`, but the index it actually read was `posPeriod + 1 = start + pos + 1`. On the second pass of `spawnNodes` (start > 0) the two diverge, so inputs like `１０．` or `ほげ４．` read one past the end of the codepoint vector and caused the crash reported in #157. Bound the lookahead against the absolute index instead. Dropped the now-dead `pos + 1` check so the condition reflects what is actually being guarded. Added regression tests for `１０．` and `ほげ４．`; both abort on the prior code under libstdc++ debug mode / ASan. Closes #157.
v4 runs on Node 20, which GitHub is forcing off the runners in September 2026. v5 runs on Node 24 and is a drop-in replacement. Surfaced as a warning annotation on the CI modernization run.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
NumericUnkMaker::checkPeriodbounded the lookahead withpos + 1 < codepoints.size()but readcodepoints[posPeriod + 1], whereposPeriod = start + pos. On the secondspawnNodespass (start > 0), the two diverge and inputs like１０．orほげ４．read one past the end of the codepoint vector — the crash Ozeki-san reported from Windows. Reproduced on Linux under libstdc++ debug mode.posPeriod + 1 < codepoints.size(). Audited the sibling helpers (checkInterfix,checkSuffix,checkPrefix,checkComma) — they already bound correctly, so the bug is isolated tocheckPeriod.１０．) and thestart > 0path (ほげ４．). Both abort on the prior code.actions/checkout@v4→v5to clear the Node 20 deprecation warning the newly-rewritten workflow surfaced.Test plan
ctest --test-dir build-asan --output-on-failurepasses 10/10 suites under AddressSanitizer + UBSan + libstdc++ debug mode.numeric_creator_testcasesmulti-digit number followed by trailing period does not crashanddigit+period preceded by non-numeric context does not crashabort onmasterbefore the fix, pass after.Closes #157.