Skip to content

Bounds-check the octal prefix skip in pm_integer_parse - #4353

Closed
fuzzyrichie wants to merge 1 commit into
ruby:mainfrom
fuzzyrichie:integer-parse-octal-prefix-bounds
Closed

fuzzyrichie wants to merge 1 commit into
ruby:mainfrom
fuzzyrichie:integer-parse-octal-prefix-bounds

Conversation

@fuzzyrichie

Copy link
Copy Markdown

Background

In pm_integer_parse, the octal case skips the leading 0 and then dereferences start to look for an o/O/_ prefix, without checking it against end:

case PM_INTEGER_BASE_OCTAL:
    start++; /* 0 */
    if (*start == '_' || *start == 'o' || *start == 'O') start++; /* o */

If the token were exactly "0", this would read one byte past the token, and past the source buffer if the token ended it.

Impact

None today. The lexer never passes pm_integer_parse such a token:

  • a bare 0 has its value computed during lexing (parser->integer.start == token->start), so pm_integer_node_create never calls pm_integer_parse for it;
  • every token that does reach the octal branch (00, 0_7, 0o17, 017r, ...) is at least two bytes long.

I checked this with an ASan build that parses exact-size heap buffers with no NUL terminator. 0, x = 0, 00, 0_, 0o, 0r, 0i, 07 and the other base prefixes are all clean before and after this change.

Fix

Check start < end before the prefix lookahead. This keeps the function in bounds for any token it's given, the same way the lexer guards its own lookahead with peek(). It has no effect on any token the lexer produces today.

src/integer.c compiles cleanly with prism's -Werror -Wconversion -Wpedantic flags.


🤖 Generated with Claude Code

After skipping the leading `0` of an octal literal, pm_integer_parse
dereferences `start` to look for an `o`/`_` prefix without checking
it against `end`.  If the token were exactly "0", that would read one
byte past the token, and past the source buffer if the token ended it.

The lexer never hands pm_integer_parse such a token today: a bare `0`
has its value computed during lexing, and every token that does reach
the octal branch is at least two bytes long.  Check `start < end`
anyway so the function stays in bounds for any token it is given,
matching how the lexer guards its own lookahead with peek().

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kddnewton

Copy link
Copy Markdown
Collaborator

No. This is adding a check for something that never happens.

@kddnewton kddnewton closed this Nov 9, 2026
@fuzzyrichie
fuzzyrichie deleted the integer-parse-octal-prefix-bounds branch November 9, 2026 23:50
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.

2 participants