[CLI] Reject non-positive or overflowing --repeat values - #280
Open
iliaal wants to merge 1 commit into
Open
Conversation
--repeat parsed its count with an unchecked atoi(), so a value below 1 wrapped the repeat counter negative and looped the request forever, and overflowing input relied on undefined atoi behavior. The value is now parsed with ZEND_STRTOL and anything that is empty, not fully numeric, below 1, or beyond INT_MAX is rejected with a usage error before any execution. Sibling audit: php_cli_server.c strtol usages parse host:port with bind errors as the failure path, and the lint-mode do_repeat path is safe now that num_repeats >= 1 is guaranteed.
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.
The CLI SAPI parsed the count for
--repeatwith an uncheckedatoi(), so a value below 1 wrappednum_repeatsnegative and made the post-execution repeat loop run forever, while overflowing input relied on undefinedatoi()behavior. The option now parses its argument withZEND_STRTOLand rejects empty, non-numeric, non-positive, or beyond-INT_MAXvalues with a usage error before any execution, like the other invalid CLI option diagnostics. A phpt regression test drivesphp --repeat=0,--repeat=-2,--repeat=abc, and an overflow value through a timeout-bounded exec and asserts the usage error; it fails on the unpatched build and passes after the fix, and the fullsapi/cli/testssuite shows zero new failures. Sibling audit: thestrtol()calls inphp_cli_server.cparse host:port strings where invalid values surface later as bind errors, and lint mode'sdo_repeatre-entry is safe now thatnum_repeats >= 1is guaranteed.