Repository navigation
Rephrase ast.literal_eval() to remove any security warranty #95588
Description
Activity
Comparing
literal_evalto unpickle seems really misleading... unpickling is straight up arbitrary code execution, which is a completely different threat than "might cause a crash".Also, I've seen lots of folks use
literal_evalon untrusted data, because that's always been its whole reason for existence. (If you have trusted data, you can justeval:-).) So I don't think just yanking that guarantee out from under people's feet is likely to fly with the community... it's effectively a major backwards-incompatible change.If you're really worried about this, maybe it should get a
max_input_length=10000argument or something, or whatever would make it safe enough for you? Do we even know whether it's actually unsafe right now?Reacted by Arno Gobbin and Nikolaus HansenWe could remove the safety warranties from our documentation, but we cannot drop the security promise for Python 3.11 and earlier. The API was documented as safe when Python 3.10 and earlier came out. We cannot just void this promise mid-air.
If you're really worried about this, maybe it should get a max_input_length=10000 argument or something, or whatever would make it safe enough for you? Do we even know whether it's actually unsafe right now?
I made a similar suggestion in our internal chat. We may have to restrict both input string length and amount of AST nodes.
Py_CompileString*would need to track nodes with an internal counter.If you're really worried about this, maybe it should get a max_input_length=10000 argument or something, or whatever would make it safe enough for you? Do we even know whether it's actually unsafe right now?
I made a similar suggestion in our internal chat. We may have to restrict both input string length and amount of AST nodes.
Py_CompileString*would need to track nodes with an internal counter.I am generally opposed to that kind of thing because is very difficult to predict correctly what the effect will be. For example, the parser creates AST nodes on the go as it parses incorrect constructs and those will not be ultimately used unless the end in a cache. What is worse, changing the grammar will have very visible effects so something that didn't crash before now will reach the limit.
Given how tricky is to maintain the parser in general I advice strongly against more complexity than we already have, which is a lot.
Every special mode of the parser can be a pain to maintain when extending error messages or doing more grammar rules
Reacted by Gregory P. Smith and Zac Hatfield-DoddsSo I don't think just yanking that guarantee out from under people's feet is likely to fly with the community... it's effectively a major backwards-incompatible change.
The documentation is wrong, misleading and should be fixed. A function which is known to crash must not be used with an untrusted string.
IMO making literal_eval() safer is a new and separated feature request. If you have a concrete idea how to make literal_eval() safer, please open a separated issue.
Also, fixing a bunch of parser issues is always welcomed, but it's also a separated issue.
The documentation is wrong, misleading and should be fixed. A function which is known to crash must not be used with an untrusted string.
The documentation is a promise to our users. We must do our best to solve the problem -- at least for common and simple cases. Just dropping the security promise from the documentation is blame shifting. It does not help our user base who is already using the feature.
Writing a more minimal expression parser as Raymond suggests is likely the best way to make
literal_eval()safe. As for a promise to our users, there are some real challenges fulfilling that and it's best to be honest if we can't fulfill it. E.g. we probably can't fixliteral_eval()to be 100% bulletproof in bug fix release. OTOH, I think it's a fairly heavily used feature so probably we should try to fix it. Even though the minimal expression parser likely adds a fair bit of maintenance work.Is there no way to figure out an acceptable solution complexity wise? Or just to build more confidence in what we have? Maybe by adding a fuzzer test for it (there seems to be a test for json here, which is probably quite similar in many ways).
A reduced complexity/more minimal expression parser version seems best in principle, but I don't know how feasible it is.We do fuzz test ast.literal_eval via Google's oss-fuzz. The entry point is https://github.com/python/cpython/blob/main/Modules/_xxtestfuzz/fuzzer.c#L396. It has revealed numerous compiler and parser issues that have been fixed. But is often stuck re-triggering known non-trivial issues such as stack overflows and computational timeouts at this point.
"The" problem with writing a minimal expression parser to replace literal_eval's implementation is the complexity.
literal_evaldoes a lot more than I suspect many users actually need or understand as the range of things Python literals encompass is large and infinitely nested.Ex:
{(15, (9.25e-16+9j)): '''str''', "8": [{(),}, ..., None, False, ()]}is a Python literal. Did your program really need to be able to accept that as input? I doubt it.We could remove the safety warranties from our documentation, but we cannot drop the security promise for Python 3.11 and earlier. The API was documented as safe when Python 3.10 and earlier came out. We cannot just void this promise mid-air.
We can and must. Because we'll never fix literal_eval in anything acceptable to backport to a release branch. Doing that requires an entire new non-recursive not Python compiler based implementation. The bug we can fix is in our documentation which claims it is safe to crash the process by parsing untrusted data. It isn't.
I doubt many will need the full power here, but things like dicts containing a list maybe, in fact a dict with a list containing tuples is the exact, untrusted, use-case, I have.
If you update the docs, maybe mention
jsonwhich has support for a large range of use-cases? For the use-case I have in mind, unfortunaely supporting tuples literals rather than only lists is the issue that json does not cover.So the problem with
literal_evalis entirely with the parser, right? the "eval" part looks pretty trivial/safe AFAICT? I thinkliteral_evalis kind of a red herring... the much more important question is whether we documentast.parseas performing arbitrary code execution. Because tons of things useast.parsebesidesliteral_eval.Concretely: suppose you have an IDE where whenever you open a
.pyfile, it does some static introspection, part of which involvesast.parse. Is this a supported use case? Or does it need to be handled with the same radioactive tongs as unpickling and eval?Likely?
ast.parseandast.literal_evalboth crash on the same thing with a stack overflow:$ ulimit -s 256 $ python Python 3.10.5 (main, Jun 8 2022, 09:26:22) [GCC 11.3.0] on linux Type "help", "copyright", "credits" or "license" for more information. >>> n = 200 >>> def s(): ... return '(' * n + '0' + ',)' * n ... >>> import ast >>> ast.parse(s()) Segmentation faultUnder a more common default stack size or >= 2048KiB our internal checks for too much nesting being > 200 fire and lead to a
SyntaxError: too many nested parenthesesinstead. So many of these crashes are likely prevented in common environments but we don't do enough to make a guarantee.We cannot control the C stack size - and introspecting it is not always plausible - so we cannot make guarantees so long as we're C stack recursive. #91079 is related to helping out here.
Fundamentally this is a problem with using recursion in a language that doesn't have a dynamic stack (read: C and C++). Input based recursion is not wise for parsing untrusted inputs in such an environment.
6 remaining items
More explicit wording added.
- added a commit that references this issue
on Oct 3, 2022 Are there any hints on a workaround(s)? The only thing I can think of right now is just rejecting "large" inputs (not sure how large). Which is still a bit inconvenient in my use-case (which at some time explicitly wanted to allow them). Rejecting deep nesting, or very many nodes would seem a bit clearer.
I can of course do that, and maybe there is just no avoiding it anyway, but "advertising" it, e.g. by having a kwarg to opt-in might be good?Are there any hints on a workaround(s)?
In my experience, the most secure way to evaluate/run untrusted code is to spawn a separated process and runs this process in a sandbox where you can limit time, memory, syscalls, etc.
Yea, but whether or not it helps me, it seem like there should be some practical solution for very simple stuff here.
- Nobody wants to sand-box evaluating a single floating point number. So there is some "threshold" of complexity that should be safe. Maybe using
jsonactually helps increase it (not for me, but generally)? (json also warns about sizes, but maybe it is more graceful in the sense that there is no sudden crash, just DoS through linear resource usage increase?). - If I use literal-eval as a library, it seems strange if I sandbox for the user (can I even do that)? But, I also cannot reasonably force everyone to sandbox. So I must add some arbitrary threshold beyond which I tell users: "Oh, maybe be careful, if you are worried you probably have to sandbox."
I would be surprised if I am the only one in a position where historically
literal_evalwas assumed to be safe (maybe sloppily so) and removing that guarantee will be bumpy.- Nobody wants to sand-box evaluating a single floating point number. So there is some "threshold" of complexity that should be safe. Maybe using
I would be surprised if I am the only one in a position where historically literal_eval was assumed to be safe (maybe sloppily so) and removing that guarantee will be bumpy.
It was never safe. The only change is the documentation that has been fixed. One way to reduce the risk of crash is to reject strings longer than a limit (ex: 100 characters).
This issue is closed. I suggest you opening a discussion elsewhere. There is no plan to implement a sandbox in CPython: https://lwn.net/Articles/574215/
- added a commit that references this issue
on Dec 3, 2024 - added a commit that references this issue
on Aug 26, 2026
Currently, ast.literal_eval() documentation gives multiple security warranties:
IMO that's plain wrong if you read the following RED WARNING:
The documentation should be rephrased to only described the purpose of the function and make it very clear that it must NOT be used on untrusted sources.
We can follow the phrasing of the pickle documentation: https://docs.python.org/dev/library/pickle.html
Linked PRs
ast.literal_evaldocs. (GH-95919) #126729