GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-95588: Drop the safety claim from `ast.literal_eval` docs. by gpshead · Pull Request #95919 · python/cpython

Conversation

@gpshead

@gpshead gpshead commented Aug 12, 2022 •
edited by bedevere-bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

It was never safe and this claim conflicts directly with the big warning in the docs about it being able to crash the interpreter.

Making it safe is not a mere bugfix but an entire rewrite, we're never going to live up to the safety claim via a bugfix and people with existing releases will always have versions around that are unsafe even if we did. So making that claim is bad for our users.

  • Issue: Rephrase ast.literal_eval() to remove any security warranty #95588

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@gpshead
Drop the safety claim from ast.literal_eval docs.
a7d0dbf
It was never safe and this claim conflicts directly with the big
warning in the docs about it being able to crash the interpreter.
@gpshead gpshead added docs Documentation in the Doc dir needs backport to 3.7 type-crash A hard crash of the interpreter, possibly with a core dump needs backport to 3.11 only security fixes labels Aug 12, 2022
@gpshead
gpshead requested a review from isidentical as a code owner August 12, 2022 08:26
@bedevere-bot bedevere-bot added the awaiting core review label Aug 12, 2022
@gpshead
add the word only as I did in the docstring.
d2632af
@gpshead
gpshead requested review from isidentical and vstinner and removed request for isidentical August 12, 2022 08:35
@gpshead gpshead self-assigned this Aug 12, 2022
@njsmith

njsmith commented Aug 12, 2022

Copy link
Copy Markdown
Contributor

Can we state clearly that maliciously large/complex inputs can cause a DoS (process crash or slow execution time), but nothing worse than that? That's a useful guarantee, and not the same as what most people think of when they think "unsafe for untrusted input".

(Can it even cause unbounded execution time? with the new parser, shouldn't literal_eval be O(n) in time and space?)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@gpshead

gpshead commented Aug 12, 2022

Copy link
Copy Markdown
Member Author

It only takes 200 bytes to crash the process so it isn't even about being what I'd call large. (which is why i removed the word "large" from the warning)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@gpshead

gpshead commented Aug 12, 2022

Copy link
Copy Markdown
Member Author

i'm weary of guaranteeing it doesn't lead to arbitrary code execution. I believe that is the intent and might be true, and any example that allowed that is one we'd care about, but it is hard to claim as the code this uses for parsing wasn't designed with untrusted input in mind because of the rhetorical "who even does that to a compiler?" question.

so the wording i have here tries to explain that it isn't an arbitrary python code execution function. but doesn't specifically say it cannot lead to other things, just that we know you can get memory exhaustion and C stack overflows.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@gpshead

gpshead commented Aug 12, 2022

Copy link
Copy Markdown
Member Author

unbounded time is possible but i can't get into the details of that in public right now. at input sizes too small to cause a stack overflow I do not believe you'll run into time issues.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

vstinner
vstinner reviewed Aug 17, 2022
View reviewed changes
Comment thread Doc/library/ast.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Doc/library/ast.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

gpshead added 2 commits October 1, 2022 18:33
@gpshead
Merge branch 'main' into gh95588-ast-doc
c1fea1d
@gpshead
Adjust wording per vstinner's review.
f01289c
@gpshead gpshead removed type-crash A hard crash of the interpreter, possibly with a core dump needs backport to 3.7 labels Oct 1, 2022
@gpshead
gpshead merged commit 8baef8a into python:main Oct 2, 2022
@miss-islington

miss-islington commented Oct 2, 2022

Copy link
Copy Markdown
Contributor

Thanks @gpshead for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.11.
🐍🍒⛏🤖

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot bedevere-bot removed the awaiting core review label Oct 2, 2022
@gpshead
gpshead deleted the gh95588-ast-doc branch October 2, 2022 00:55
@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label Oct 2, 2022
@bedevere-bot

bedevere-bot commented Oct 2, 2022

Copy link
Copy Markdown

GH-97714 is a backport of this pull request to the 3.11 branch.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot

bedevere-bot commented Oct 2, 2022

Copy link
Copy Markdown

GH-97715 is a backport of this pull request to the 3.10 branch.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot bedevere-bot removed the needs backport to 3.10 label Oct 2, 2022
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 2, 2022
@gpshead @miss-islington
pythongh-95588: Drop the safety claim from ast.literal_eval docs. (p…
7cb38c5
…ythonGH-95919)

It was never really safe and this claim conflicts directly with the big warning in the docs about it being able to crash the interpreter.
(cherry picked from commit 8baef8a)

Co-authored-by: Gregory P. Smith <[email protected]>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 2, 2022
@gpshead @miss-islington
pythongh-95588: Drop the safety claim from ast.literal_eval docs. (p…
2a2301e
…ythonGH-95919)

It was never really safe and this claim conflicts directly with the big warning in the docs about it being able to crash the interpreter.
(cherry picked from commit 8baef8a)

Co-authored-by: Gregory P. Smith <[email protected]>
miss-islington added a commit that referenced this pull request Oct 2, 2022
@miss-islington @gpshead
gh-95588: Drop the safety claim from ast.literal_eval docs. (GH-95919)
9189cd6
It was never really safe and this claim conflicts directly with the big warning in the docs about it being able to crash the interpreter.
(cherry picked from commit 8baef8a)

Co-authored-by: Gregory P. Smith <[email protected]>
miss-islington added a commit that referenced this pull request Oct 2, 2022
@miss-islington @gpshead
gh-95588: Drop the safety claim from ast.literal_eval docs. (GH-95919)
a4fbb94
It was never really safe and this claim conflicts directly with the big warning in the docs about it being able to crash the interpreter.
(cherry picked from commit 8baef8a)

Co-authored-by: Gregory P. Smith <[email protected]>
serhiy-storchaka pushed a commit to serhiy-storchaka/cpython that referenced this pull request Oct 2, 2022
@gpshead @serhiy-storchaka
pythongh-95588: Drop the safety claim from ast.literal_eval docs. (p…
5ffc011
…ython#95919)

It was never really safe and this claim conflicts directly with the big warning in the docs about it being able to crash the interpreter.
@vstinner vstinner mentioned this pull request Oct 3, 2022
Rephrase ast.literal_eval() to remove any security warranty #95588
Closed
carljm added a commit to carljm/cpython that referenced this pull request Oct 3, 2022
@carljm
Merge branch 'main' into dictwatch
e83abac
* main: (2069 commits)
  pythongh-96512: Move int_max_str_digits setting to PyConfig (python#96944)
  pythongh-94808: Coverage: Check picklablability of calliter (python#95923)
  pythongh-94808: Add test coverage for PyObject_HasAttrString (python#96627)
  pythongh-94732: Fix KeyboardInterrupt race in asyncio run_forever() (python#97765)
  Fix typos in `bltinmodule.c`. (pythonGH-97766)
  pythongh-94808: `_PyLineTable_StartsLine` was not used (pythonGH-96609)
  pythongh-97681: Remove Tools/demo/ directory (python#97682)
  Fix typo in unittest docs (python#97742)
  pythongh-97728: Argument Clinic: Fix uninitialized variable in the Py_UNICODE converter (pythonGH-97729)
  pythongh-95913: Fix PEP number in PEP 678 What's New ref label (python#97739)
  pythongh-95913: Copyedit/improve New Modules What's New section (python#97721)
  pythongh-97740: Fix bang in Sphinx C domain ref target syntax (python#97741)
  pythongh-96819: multiprocessing.resource_tracker: check if length of pipe write <= 512 (python#96890)
  pythongh-97706: multiprocessing tests: Delete unused variable `rand` (python#97707)
  pythonGH-85447: Clarify docs about awaiting future multiple times (python#97738)
  [docs] Update logging cookbook with recipe for using a logger like an output… (pythonGH-97730)
  pythongh-97607: Fix content parsing in the impl-detail reST directive (python#97652)
  pythongh-95975: Move except/*/finally ref labels to more precise locations (python#95976)
  pythongh-97591: In `Exception.__setstate__()` acquire strong references before calling `tp_hash` slot (python#97700)
  pythongh-95588: Drop the safety claim from `ast.literal_eval` docs. (python#95919)
  ...
@slateny slateny mentioned this pull request Dec 20, 2022
gh-100305: Deemphasize that ast.literal_eval is safe in eval documentation #100326
Merged
@Eclips4 Eclips4 mentioned this pull request Nov 12, 2024
ast.literal_eval Segmentation Fault in Python 3.9/3.10 #126711
Closed
@skirpichev

skirpichev commented Nov 12, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Lets try to backport this on 3.9 (see #126711).

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@skirpichev skirpichev added the needs backport to 3.9 label Nov 12, 2024
@miss-islington-app

miss-islington-app Bot commented Nov 12, 2024

Copy link
Copy Markdown

Thanks @gpshead for the PR 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒⛏🤖

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 12, 2024
@gpshead @miss-islington
pythongh-95588: Drop the safety claim from ast.literal_eval docs. (p…
d276b5a
…ythonGH-95919)

It was never really safe and this claim conflicts directly with the big warning in the docs about it being able to crash the interpreter.
(cherry picked from commit 8baef8a)

Co-authored-by: Gregory P. Smith <[email protected]>
@bedevere-app

bedevere-app Bot commented Nov 12, 2024

Copy link
Copy Markdown

GH-126729 is a backport of this pull request to the 3.9 branch.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app bedevere-app Bot removed the needs backport to 3.9 label Nov 12, 2024
ambv pushed a commit that referenced this pull request Dec 3, 2024
@miss-islington @gpshead
[3.9] gh-95588: Drop the safety claim from ast.literal_eval docs. (G…
08830c7
…H-95919) (GH-126729)

It was never really safe and this claim conflicts directly with the big warning in the docs about it being able to crash the interpreter.
(cherry picked from commit 8baef8a)

Co-authored-by: Gregory P. Smith <[email protected]>
gentoo-bot pushed a commit to gentoo/cpython that referenced this pull request Apr 9, 2025
@miss-islington @gpshead @mgorny
[3.9] pythongh-95588: Drop the safety claim from ast.literal_eval d…
953a502
…ocs. (pythonGH-95919) (pythonGH-126729)

It was never really safe and this claim conflicts directly with the big warning in the docs about it being able to crash the interpreter.
(cherry picked from commit 8baef8a)

Co-authored-by: Gregory P. Smith <[email protected]>
@coderabbitai coderabbitai Bot mentioned this pull request Jul 23, 2026
Massive fix autotest/tp-libvirt#6915
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vstinner vstinner vstinner left review comments
@isidentical isidentical Awaiting requested review from isidentical isidentical is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@gpshead gpshead

Labels

docs Documentation in the Doc dir

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

6 participants

@gpshead @njsmith @miss-islington @bedevere-bot @skirpichev @vstinner
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.