Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

assertion failure in Parser/string_parser.c #90661

Closed
gpshead opened this issue Jan 24, 2022 · 10 comments
Closed

assertion failure in Parser/string_parser.c #90661

gpshead opened this issue Jan 24, 2022 · 10 comments
Assignees
Labels
3.11 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@gpshead
Copy link
Member

gpshead commented Jan 24, 2022

BPO 46503
Nosy @gpshead, @ericvsmith, @lysnikolaou, @pablogsal, @miss-islington
PRs
  • bpo-46503: Prevent an assert from firing when parsing some invalid \N sequences in f-strings. #30865
  • [3.10] bpo-46503: Prevent an assert from firing when parsing some invalid \N sequences in f-strings. (GH-30865) #30866
  • [3.9] bpo-46503: Prevent an assert from firing when parsing some invalid \N sequences in f-strings. (GH-30865) #30867
  • bpo-46503: Test for assert failures. #30883
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/ericvsmith'
    closed_at = <Date 2022-01-25.03:14:13.522>
    created_at = <Date 2022-01-24.18:30:42.762>
    labels = ['interpreter-core', 'type-crash', '3.11']
    title = 'assertion failure in Parser/string_parser.c'
    updated_at = <Date 2022-01-29.03:21:39.392>
    user = 'https://github.com/gpshead'

    bugs.python.org fields:

    activity = <Date 2022-01-29.03:21:39.392>
    actor = 'eric.smith'
    assignee = 'eric.smith'
    closed = True
    closed_date = <Date 2022-01-25.03:14:13.522>
    closer = 'eric.smith'
    components = ['Parser']
    creation = <Date 2022-01-24.18:30:42.762>
    creator = 'gregory.p.smith'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 46503
    keywords = ['patch']
    message_count = 10.0
    messages = ['411503', '411510', '411511', '411531', '411548', '411549', '411550', '411551', '411593', '412048']
    nosy_count = 5.0
    nosy_names = ['gregory.p.smith', 'eric.smith', 'lys.nikolaou', 'pablogsal', 'miss-islington']
    pr_nums = ['30865', '30866', '30867', '30883']
    priority = 'high'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'crash'
    url = 'https://bugs.python.org/issue46503'
    versions = ['Python 3.11']

    @gpshead
    Copy link
    Member Author

    gpshead commented Jan 24, 2022

    >>> v = r"""f'\N  '"""
    >>> import ast
    >>> ast.literal_eval(v)
    python: ../gpshead/Parser/string_parser.c:487: fstring_find_literal: Assertion `s == end || *s == '{' || *s == '}'' failed.
    Aborted
    

    this comes from oss-fuzz after enabling assert checks in its cpython builds. :)

    https://oss-fuzz.com/testcase-detail/4805529363415040 & https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=43730 for those who have access.

    @gpshead gpshead added 3.11 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-crash A hard crash of the interpreter, possibly with a core dump labels Jan 24, 2022
    @pablogsal
    Copy link
    Member

    Adding Eric as this seems to be in the f-string parser.

    @ericvsmith ericvsmith assigned ericvsmith and unassigned pablogsal Jan 24, 2022
    @ericvsmith
    Copy link
    Member

    I'll take a look.

    @ericvsmith
    Copy link
    Member

    This triggers the same problem:
    f'\N '
    ast.literal_eval() isn't needed.

    I think it's just the assert that's wrong, but I'm still checking.

    @ericvsmith
    Copy link
    Member

    Note that f'\N ' (with a single space) isn't enough to trigger this behavior. It requires at least two characters after the '\N'. The first is when the invalid string is recognized, and it's the presence of the second character that triggers the failed assert.

    @ericvsmith
    Copy link
    Member

    New changeset 0daf721 by Eric V. Smith in branch 'main':
    bpo-46503: Prevent an assert from firing when parsing some invalid \N sequences in f-strings. (GH-30865)
    0daf721

    @ericvsmith
    Copy link
    Member

    New changeset c314e3e by Miss Islington (bot) in branch '3.9':
    bpo-46503: Prevent an assert from firing when parsing some invalid \N sequences in f-strings. (GH-30865) (30867)
    c314e3e

    @ericvsmith
    Copy link
    Member

    New changeset 894e8c1 by Miss Islington (bot) in branch '3.10':
    bpo-46503: Prevent an assert from firing when parsing some invalid \N sequences in f-strings. (GH-30865) (GH-30866)
    894e8c1

    @pablogsal
    Copy link
    Member

    Thanks for the quick fix, Eric!

    @ericvsmith
    Copy link
    Member

    In case anyone cares: in a non-debug build, this error had no real effect. It just caused the "find the literal part of an fstring" routine to terminate early, but since the part that it had already identified was still in error, a syntax error was still raised.

    For "\Nxy" it would terminate at "\Nx", instead of consuming the whole string. But since "\Nx" isn't a valid string (bad unicode name escape), it would raise the same syntax error as "\Nxy".

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.11 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-crash A hard crash of the interpreter, possibly with a core dump
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants