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

ast.literal_eval Segmentation Fault in Python 3.9/3.10 #126711

Closed
EgodPrime opened this issue Nov 12, 2024 · 10 comments
Closed

ast.literal_eval Segmentation Fault in Python 3.9/3.10 #126711

EgodPrime opened this issue Nov 12, 2024 · 10 comments
Labels
3.9 only security fixes docs Documentation in the Doc dir type-security A security issue

Comments

@EgodPrime
Copy link

EgodPrime commented Nov 12, 2024

Crash report

What happened?

l  = 200000
a = 'P/a'*l
import ast
ast.literal_eval(a)

The above code makes Python 3.9/3.10 crash with Segmentation Fault.

This error input can be correctly detected in Python 3.11+ with RecursionError: maximum recursion depth exceeded during ast construction, but it still results into crash in 3.9.20 and 3.10.15.

A smaller l such as 200 will get the correct error Value Error: malformed node or string for all Python 3.9+

CPython versions tested on:

3.9, 3.10

Operating systems tested on:

Linux

Output from running 'python -VV' on the command line:

Python 3.9.20 (main, Oct 3 2024, 07:27:41) [GCC 11.2.0], Python 3.10.15 (main, Oct 3 2024, 07:27:34) [GCC 11.2.0]

@EgodPrime EgodPrime added the type-crash A hard crash of the interpreter, possibly with a core dump label Nov 12, 2024
@zware zware added 3.10 only security fixes 3.9 only security fixes labels Nov 12, 2024
@skirpichev
Copy link
Member

skirpichev commented Nov 12, 2024

@EgodPrime, please avoid creating duplicate issues.

FWIW, the 3.9 docs says: "It is possible to crash the Python interpreter with a sufficiently large/complex string due to stack depth limitations in Python’s AST compiler."

3.10+ docs are more vague: "It is possible to crash the Python interpreter due to stack depth limitations in Python’s AST compiler."

So, I'm not sure if we should count this as a bug.

@skirpichev skirpichev added the pending The issue will be closed if no feedback is provided label Nov 12, 2024
@EgodPrime
Copy link
Author

@skirpichev, thanks for your reply. But I am wondering whether it means that Python 3.9/3.10, which is still in use in production, is not safe for such inputs.

@skirpichev
Copy link
Member

Documentation (since 3.10) explicitly says, that ast.litaral_eval() is not safe for arbitrary input: "This function had been documented as “safe” in the past without defining what that meant. That was misleading. This is specifically designed not to execute Python code, unlike the more general eval(). There is no namespace, no name lookups, or ability to call out. But it is not free from attack: A relatively small input can lead to memory exhaustion or to C stack exhaustion, crashing the process. There is also the possibility for excessive CPU consumption denial of service on some inputs. Calling it on untrusted data is thus not recommended."

@EgodPrime
Copy link
Author

I see it. And I now think this is not a bug but just a poc to trigger the crash (C stack exhaustion) of the officially declared unsafe ast.literal_eval.

@Eclips4
Copy link
Member

Eclips4 commented Nov 12, 2024

Hello! Thank you for the report. Since ast.literal_eval has been documented as a function that can potentially lead to interpreter crashes, I don't think it's worthwhile to fix it in 3.9. However, it seems reasonable to me to backport #95919 to 3.9 version. cc @gpshead

EDIT: I saw a Christian comment: #95588 (comment). Though, I can't say that I agree with it. This function was probably never safe and the documentation was just wrong.

@Eclips4 Eclips4 removed the pending The issue will be closed if no feedback is provided label Nov 12, 2024
@skirpichev
Copy link
Member

I'll prepare a patch.

@skirpichev skirpichev added docs Documentation in the Doc dir and removed 3.10 only security fixes type-crash A hard crash of the interpreter, possibly with a core dump labels Nov 12, 2024
@skirpichev
Copy link
Member

Well, 3.9 is for security-fixes only. But here is a docs backport: #126729

@Eclips4 Eclips4 added the type-security A security issue label Nov 12, 2024
@Eclips4
Copy link
Member

Eclips4 commented Nov 12, 2024

I would like to consider this issue as a security problem because crash of interpreter can lead to DOS.

@ZeroIntensity
Copy link
Member

My main concern here is that we can't undo documenting ast.literal_eval as safe. Plenty of people are using it in seemingly unsafe places as a result, and I'm not sure it's a good idea to just stick a big bandaid on it for 3.9. If there's absolutely nothing we can do to fix it, then perhaps we could emit a warning when using it on 3.9?

@gpshead
Copy link
Member

gpshead commented Nov 12, 2024

#126711 already backports the doc change to 3.9. So yes we can. We can never prevent people from writing code that doesn't behave as they desire, all we can do is make expectations more clear.

We cannot meaningfully add a warning no matter what branch because there are plenty of valid uses of ast.literal_eval and it is impossible to tell which ones are and are not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.9 only security fixes docs Documentation in the Doc dir type-security A security issue
Projects
Status: Todo
Development

No branches or pull requests

7 participants