[Bug] Null Pointer Dereference in parser.c — `first=1` path of 4 collection-entry functions

[BoxStrikesTeam]

Summary

Four functions in src/parser.c call PEEK_TOKEN(parser) inside a if (first) block and immediately dereference the result without checking for NULL. PEEK_TOKEN can return NULL on a read or memory error. When this happens, the next line accesses token->start_mark, causing a null pointer dereference / segfault.

Affected Version

v0.2.6-rc.1 (latest commit)

Affected File & Lines

File: src/parser.c

Function	Line	Vulnerable code
yaml_parser_parse_block_sequence_entry	740–741	token = PEEK_TOKEN(parser); → token->start_mark
yaml_parser_parse_block_mapping_key	848–849	token = PEEK_TOKEN(parser); → token->start_mark
yaml_parser_parse_flow_sequence_entry	963–964	token = PEEK_TOKEN(parser); → token->start_mark
yaml_parser_parse_flow_mapping_key	1119–1120	token = PEEK_TOKEN(parser); → token->start_mark

Root Cause

PEEK_TOKEN is defined as (parser.c line 48):

#define PEEK_TOKEN(parser)                                                      \
    ((parser->token_available || yaml_parser_fetch_more_tokens(parser)) ?       \
        parser->tokens.head : NULL)

It returns NULL when yaml_parser_fetch_more_tokens fails (e.g. read error, memory allocation failure, or malformed input that causes the scanner to return early).

All four functions follow the same unsafe pattern inside their if (first) branch:

if (first) {
    token = PEEK_TOKEN(parser);          // ← can return NULL
    if (!PUSH(parser, parser->marks, token->start_mark))  // ← NPD if token == NULL
        return 0;
    SKIP_TOKEN(parser);
}

Notably, every other PEEK_TOKEN call in these same functions is guarded correctly:

token = PEEK_TOKEN(parser);
if (!token) return 0;   // ← guard present everywhere else

The first branch is the sole exception.

Contrast with correct usage

The very next PEEK_TOKEN call in each function (outside the if (first) block) is properly guarded:

// yaml_parser_parse_block_sequence_entry
if (first) {
    token = PEEK_TOKEN(parser);                           // NO check ← BUG
    if (!PUSH(parser, parser->marks, token->start_mark))
        return 0;
    SKIP_TOKEN(parser);
}
 
token = PEEK_TOKEN(parser);
if (!token) return 0;                                    // check present ← correct

How to trigger

The first=1 path is taken the very first time each collection is entered. A truncated or malformed YAML stream that causes the scanner to fail (return error / EOF) during that first token fetch will hit the NULL dereference before any graceful error handling occurs.

Minimal malformed YAML examples likely to exercise each path:

# Block sequence (triggers yaml_parser_parse_block_sequence_entry)
- [truncated here
 
# Block mapping (triggers yaml_parser_parse_block_mapping_key)
key: [truncated here
 
# Flow sequence (triggers yaml_parser_parse_flow_sequence_entry)
[truncated
 
# Flow mapping (triggers yaml_parser_parse_flow_mapping_key)
{truncated

Paired with a read handler that returns an error mid-stream, any of these will trigger the bug.

Suggested Fix

Add a NULL guard immediately after each PEEK_TOKEN call inside the if (first) block, consistent with every other call site in the file:

// Before (buggy):
if (first) {
    token = PEEK_TOKEN(parser);
    if (!PUSH(parser, parser->marks, token->start_mark))
        return 0;
    SKIP_TOKEN(parser);
}
 
// After (fixed):
if (first) {
    token = PEEK_TOKEN(parser);
    if (!token) return 0;          // ← add this guard
    if (!PUSH(parser, parser->marks, token->start_mark))
        return 0;
    SKIP_TOKEN(parser);
}

This one-line fix must be applied to all four functions:

• yaml_parser_parse_block_sequence_entry (line ~741)
• yaml_parser_parse_block_mapping_key (line ~849)
• yaml_parser_parse_flow_sequence_entry (line ~964)
• yaml_parser_parse_flow_mapping_key (line ~1120)

Environment

• Library: libyaml master branch
• File: src/parser.c
• Compiler: Any C compiler (bug is logic-level, not platform-specific

[perlpunk]

Thanks for the report.

I checked all four code locations and tested with your examples, e.g.

echo -n '- [' | ./tests/run-parser-test-suite

In all four cases parser->token_available is returning true, meaning that there is already a token in the queue.

Since PEEK_TOKEN does

parser->token_available || yaml_parser_fetch_more_tokens(parser)

it will never call yaml_parser_fetch_more_tokens in those cases.

(Of course it might still be a good thing to check token in all cases.)

But maybe I'm missing something. Could you help me with a reproducer?

[BoxStrikesTeam]

Thank you for taking the time to investigate and test this carefully.

After re-reading the code with your feedback in mind, I agree — this is not a real bug.

The key insight I missed: token_available is only set to 1 at the end of yaml_parser_fetch_more_tokens(), and only when it returns successfully (line 857 in scanner.c). This means that when PEEK_TOKEN short-circuits on parser->token_available being true, tokens.head is guaranteed to be a valid, non-NULL pointer. There is no code path where token_available == true and tokens.head == NULL at the same time.

So in the if (first) branch, PEEK_TOKEN cannot return NULL — the invariant is maintained by the scanner.

I was wrong to report this as a null pointer dereference. The code is correct.

That said, I think the observation you made at the end of your message is worth keeping in mind:

Of course it might still be a good thing to check token in all cases.

Adding a NULL guard in the if (first) branch would make the invariant explicit and protect against future changes to the scanner that might break it — but that is a code style/robustness suggestion, not a bug fix.

Apologies for the noise, and thank you again for the thorough response.