Skip to content

Stale parser->previous after parse_heredoc causes unsigned location length underflow on declaration line #4359

Open

Description

@stevenjohnstone

Fuzzing find on bdbd482.

When parse_heredoc finishes parsing a heredoc body, it rewinds parser->current back to the declaration line (immediately after the heredoc opening token, e.g. <<0), but leaves parser->previous set to PM_TOKEN_HEREDOC_END at the end of the heredoc body (at a higher byte offset than parser->current).

If the caller then synthesizes a missing token via expect1 (which uses parser->previous.end) before consuming another token on the declaration line, the synthesized token is given a byte offset at the end of the heredoc body while subsequent tokens on the declaration line have smaller byte offsets, causing start > end and underflowing uint32_t location.length in the enclosing AST node.

Reproduction

Simulation here: https://stevenjohnstone.github.io/prism-loupe/?version=v2.1.0&code=Ly8%2FPDwwIGEg.

✗ ./bin/parse -e "//?<<0 a "
Errors:
[#<Prism::ParseError @type=:heredoc_term @message="unterminated heredoc; can't find string \"0\" anywhere before EOF" @location=#<Prism::Location @start_offset=5 @length=1 start_line=1> @level=:syntax>,
 #<Prism::ParseError @type=:heredoc_term @message="unterminated heredoc; can't find string \"0\" anywhere before EOF" @location=#<Prism::Location @start_offset=5 @length=1 start_line=1> @level=:syntax>,
 #<Prism::ParseError @type=:ternary_colon @message="expected a `:` after the true expression of a ternary operator" @location=#<Prism::Location @start_offset=9 @length=0 start_line=1> @level=:syntax>]

AST:
@ ProgramNode (location: (1,0)-(1,8))
├── flags: ∅
├── locals: []
└── statements:
    @ StatementsNode (location: (1,0)-(1,8))
    ├── flags: ∅
    └── body: (length: 1)
        └── @ IfNode (location: (1,0)-(1,8))
            ├── flags: newline
            ├── if_keyword_loc: ∅
            ├── predicate:
            │   @ MatchLastLineNode (location: (1,0)-(1,2))
            │   ├── flags: static_literal, forced_us_ascii_encoding
            │   ├── opening_loc: (1,0)-(1,1) = "/"
            │   ├── content_loc: (1,1)-(1,1) = ""
            │   ├── closing_loc: (1,1)-(1,2) = "/"
            │   └── unescaped: ""
            ├── then_keyword_loc: (1,2)-(1,3) = "?"
            ├── statements:
            │   @ StatementsNode (location: (1,3)-(1,6))
            │   ├── flags: ∅
            │   └── body: (length: 1)
            │       └── @ StringNode (location: (1,3)-(1,6))
            │           ├── flags: newline
            │           ├── opening_loc: (1,3)-(1,6) = "<<0"
            │           ├── content_loc: (1,9)-(1,9) = ""
            │           ├── closing_loc: ∅
            │           └── unescaped: ""
            ├── subsequent:
            │   @ ElseNode (location: (1,9)-(1,4294967304))
            │   ├── flags: ∅
            │   ├── else_keyword_loc: (1,9)-(1,9) = ""
            │   ├── statements:
            │   │   @ StatementsNode (location: (1,7)-(1,8))
            │   │   ├── flags: ∅
            │   │   └── body: (length: 1)
            │   │       └── @ CallNode (location: (1,7)-(1,8))
            │   │           ├── flags: newline, variable_call, ignore_visibility
            │   │           ├── receiver: ∅
            │   │           ├── call_operator_loc: ∅
            │   │           ├── name: :a
            │   │           ├── message_loc: (1,7)-(1,8) = "a"
            │   │           ├── opening_loc: ∅
            │   │           ├── arguments: ∅
            │   │           ├── closing_loc: ∅
            │   │           ├── equal_loc: ∅
            │   │           └── block: ∅
            │   └── end_keyword_loc: ∅
            └── end_keyword_loc: ∅

The ElseNode created in parse_ternary is given start = 9 (EOF) and end = 8 (end of a), wrapping uint32_t location.length (U32(end - start)) to 4294967295 (0xFFFFFFFF). Ruby adds then calculates end_offset=9+4294967295=4294967304(232+8).

Root Cause

  1. In "//?<<0 a ", //? starts a ternary conditional expression (parse_ternary).
  2. The true-branch expression is the heredoc <<0 (offsets 3..6).
  3. parse_heredoc consumes PM_TOKEN_HEREDOC_START (opening, offsets 3..6), lexes the heredoc body up to EOF (offset 9), and consumes PM_TOKEN_HEREDOC_END at 9..9.
  4. Consuming PM_TOKEN_HEREDOC_END sets parser->previous to 9..9, while rewinding parser->current back to offset 6 on line 1 (PM_TOKEN_IDENTIFIER "a" at 7..8).
  5. Back in parse_ternary, the parser expects : (PM_TOKEN_COLON) after the true-branch <<0:
    expect1(parser, PM_TOKEN_COLON, PM_ERR_TERNARY_COLON);
    pm_token_t colon = parser->previous;
    Because parser->current is "a" (7..8) rather than :, expect1 fails and synthesizes the missing : token at parser->previous.end — which is still 9 (PM_TOKEN_HEREDOC_END at EOF) instead of 6 (the end of <<0).
  6. Next, parse_ternary parses the false-branch expression "a" at 7..8 and calls pm_else_node_create spanning from colon.start (9) to the end of "a" (8). Since 9 > 8, end - start underflows uint32_t to 4294967295.

Impact

  • Any syntax error immediately following a heredoc on the same declaration line that synthesizes a missing token via expect1 (such as a missing : in a ternary expression, a missing ) or ], or a missing => in a hash) produces an AST node whose start is at the end of the heredoc body and whose end is on the earlier declaration line (start > end), underflowing uint32_t location.length to ~4 GB and causing out-of-bounds heap reads in C consumers such as pm_prettyprint.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions