-
-
Notifications
You must be signed in to change notification settings - Fork 30.1k
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
gh-125010: Fix use-after-free
in AST repr()
#125015
Conversation
tomasr8
commented
Oct 5, 2024
•
edited by bedevere-app
bot
Loading
edited by bedevere-app
bot
- Issue: Heap-use-after-free READ 8 · dictkeys_decref in fuzz_ast_literal_eval #125010
Could you add a test case for that? I think testing that the initial reproducer doesn't crash would be enough |
Yep, but maybe I'll try to make the reproducer a bit smaller first, it's quite big 😅 : >>> s = open("reproducer.txt").read()
>>> len(s)
782813 |
Here's how it can be reduced to avoid using the file: import ast
repro = "{0x0" + "e" * 250_000 + "%" + "e" * 250_000 + "1j}"
print(ast.literal_eval(repro)) |
Test added :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The bug doesn't really have anything to do with ast.literal_eval
, so I think the test would be clearer if it didn't use ast.literal_eval
. I can trigger a crash with a simpler repro that only uses ast.parse
(and with a shorter source snippet passed to ast.parse()
):
~/dev/cpython (main)⚡ [134] % ./python.exe -c 'import ast; repr(ast.parse("0x0" + "e" * 4_000, mode="eval"))'
./Include/refcount.h:474: _Py_NegativeRefcount: Assertion failed: object has negative ref count
<object at 0x1398df420 is freed>
Fatal Python error: _PyObject_AssertFailed: _PyObject_AssertFailed
Python runtime state: initialized
ValueError: Exceeds the limit (4300 digits) for integer string conversion; use sys.set_int_max_str_digits() to increase the limit
Current thread 0x00000001f39d4f40 (most recent call first):
File "<string>", line 1 in <module>
zsh: abort ./python.exe -c
Ah, in fact, here's a repro without even ~/dev/cpython (main)⚡ % ./python.exe -c 'import ast; repr(ast.Constant(value=eval("0x0" + "e" * 4_000)))'
./Include/refcount.h:474: _Py_NegativeRefcount: Assertion failed: object has negative ref count
<object at 0x15a858220 is freed>
Fatal Python error: _PyObject_AssertFailed: _PyObject_AssertFailed
Python runtime state: initialized
ValueError: Exceeds the limit (4300 digits) for integer string conversion; use sys.set_int_max_str_digits() to increase the limit
Current thread 0x00000001f39d4f40 (most recent call first):
File "<string>", line 1 in <module>
zsh: abort ./python.exe -c |
I simplified the test to use Alex's smaller reproducer and removed the note about |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! The test looks great and I confirmed it segfaults on main
. I'll let one of the AST experts subscribed to this PR approve and merge, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thank you Tomas.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
Misc/NEWS.d/next/Core_and_Builtins/2024-10-06-00-49-37.gh-issue-125010.gGlhaj.rst
Outdated
Show resolved
Hide resolved
If @JelleZijlstra has no further comments, I'll merge this tomorrow. |