parse: don't dereference the end of the token vector in a media list - #42
Open
metan-ucw wants to merge 1 commit into
Open
parse: don't dereference the end of the token vector in a media list#42metan-ucw wants to merge 1 commit into
metan-ucw wants to merge 1 commit into
Conversation
css__mq_parse_media_list() goes round the loop again after a trailing comma, so mq_parse_media_query() can be entered with the vector already exhausted. Its first peek then returns NULL, and while tokenIsChar() tolerates that, the `else if` beside it reads token->type straight off the NULL. A stylesheet ending in an unterminated `@media),` is enough: the `)` fails the first query, the `,` sends the list round once more, and there is nothing left to parse. Guard the branch and fall through to the iterate below, which reports CSS_INVALID — i.e. `not all`, the error handling MQ4 3.2 asks for. The three cases added to mq.dat all segfaulted before this, and differ in how the first query ends, which is what decides whether the list loops at all — `@media,` and `@media not,` do not reach the bug because that query consumes the comma itself: @media), the query fails on the ')', leaving the comma @media ),,, several empty iterations in a row @media (color), the first query is a condition, and succeeds All three now parse to `not all` (the third keeps its condition, so it dumps as `all`). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Cyril Hrubis <metan@ucw.cz>
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.
css__mq_parse_media_list() goes round the loop again after a trailing comma, so mq_parse_media_query() can be entered with the vector already exhausted. Its first peek then returns NULL, and while tokenIsChar() tolerates that, the
else ifbeside it reads token->type straight off the NULL.A stylesheet ending in an unterminated
@media),is enough: the)fails the first query, the,sends the list round once more, and there is nothing left to parse. Guard the branch and fall through to the iterate below, which reports CSS_INVALID — i.e.not all, the error handling MQ4 3.2 asks for.The three cases added to mq.dat all segfaulted before this, and differ in how the first query ends, which is what decides whether the list loops at all —
@media,and@media not,do not reach the bug because that query consumes the comma itself:@media), the query fails on the ')', leaving the comma
@media ),,, several empty iterations in a row
@media (color), the first query is a condition, and succeeds
All three now parse to
not all(the third keeps its condition, so it dumps asall).