Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

gh-118633: Add warning regarding the unsafe usage of eval and exec #118437

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Oct 30, 2024
Merged

gh-118633: Add warning regarding the unsafe usage of eval and exec #118437

merged 7 commits into from
Oct 30, 2024

Conversation

DanielRuf
Copy link
Contributor

@DanielRuf DanielRuf commented Apr 30, 2024 •
edited by bedevere-app bot
Loading

@ghost
Copy link

ghost commented Apr 30, 2024 •
edited by ghost
Loading

All commit authors signed the Contributor License Agreement.
CLA signed

@bedevere-app bedevere-app bot added docs Documentation in the Doc dir skip news awaiting review labels Apr 30, 2024
@DanielRuf
Copy link
Contributor Author

For more context, Liran Tal from Snyk posted the following on LinkedIn: https://snyk.io/de/blog/code-injection-vulnerabilities-caused-by-generative-ai/

So I opened this PR to discuss and improve the documentation concerning this matter.

@DanielRuf
Copy link
Contributor Author

DanielRuf commented Apr 30, 2024 •
edited
Loading

Should there also be some warning for exec?

@Eclips4
Copy link
Member

Eclips4 commented Apr 30, 2024

FYI, There's already an entry in faq/programming.

@DanielRuf
Copy link
Contributor Author

@Eclips4 thanks for the hint, but in my opinion this is not sufficient. Take a look at these:

https://www.php.net/manual/en/function.eval.php
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/eval

I doubt that anyone reading a documentation entry knows, that there is a separate page with an important information. The warning should be directly in the documentation.

@JelleZijlstra JelleZijlstra added the type-security A security issue label May 2, 2024
@JelleZijlstra
Copy link
Member

Agree that exec() and eval() should carry big red warnings in the docs; it's a bit surprising that they don't, when the considerably safer ast.literal_eval does (https://docs.python.org/3.13/library/ast.html#ast.literal_eval).

Please add a warning to both eval() and exec(). Also, please open an issue to link this PR to: usually that's not necessary for docs fixes, but I think this is important enough that it's good to add a bit more visibility.

@DanielRuf
Copy link
Contributor Author

I've added now the warning for exec too. Creating the issue now.

@DanielRuf DanielRuf mentioned this pull request May 6, 2024
Closed
@terryjreedy terryjreedy changed the title Add warning regarding the unsafe usage of eval gh-118633: Add warning regarding the unsafe usage of eval May 6, 2024
hugovk
hugovk reviewed May 6, 2024
View reviewed changes
Doc/library/functions.rst Outdated Show resolved Hide resolved
@DanielRuf DanielRuf changed the title gh-118633: Add warning regarding the unsafe usage of eval gh-118633: Add warning regarding the unsafe usage of eval and exec May 6, 2024
@Eclips4
Copy link
Member

Eclips4 commented May 6, 2024 •
edited
Loading

LG for me. However there is one thing that I want to discuss: Do we need to add a similar note for the compile function?

@hugovk hugovk added the needs backport to 3.12 only security fixes label May 6, 2024
JelleZijlstra
JelleZijlstra reviewed May 7, 2024
View reviewed changes
Doc/library/functions.rst Outdated Show resolved Hide resolved
Doc/library/functions.rst Outdated Show resolved Hide resolved
DanielRuf and others added 2 commits May 7, 2024 08:41
@DanielRuf @JelleZijlstra
Use suggested shorter text
2d37c80 
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
@DanielRuf @JelleZijlstra
Use suggested shorter text
3293768 
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
eli-schwartz
eli-schwartz reviewed May 7, 2024
View reviewed changes
Doc/library/functions.rst Outdated Show resolved Hide resolved
@serhiy-storchaka serhiy-storchaka added the needs backport to 3.13 bugs and security fixes label May 9, 2024
nineteendo
nineteendo reviewed Jun 2, 2024
View reviewed changes
Doc/library/functions.rst Show resolved Hide resolved
@willingc willingc enabled auto-merge (squash) October 30, 2024 00:23
@willingc
Copy link
Contributor

Thanks everyone for the PR, reviews and suggestions. I'm planning to merge.

@willingc willingc disabled auto-merge October 30, 2024 00:27
@willingc willingc enabled auto-merge (squash) October 30, 2024 00:33
@willingc willingc merged commit 00e5ec0 into python:main Oct 30, 2024
30 checks passed
@miss-islington-app
Copy link

Thanks @DanielRuf for the PR, and @willingc for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 30, 2024
@DanielRuf @Eclips4
@JelleZijlstra @miss-islington
pythongh-118633: Add warning regarding the unsafe usage of eval and e…
47d90ad 
…xec (pythonGH-118437)

* Add warning regarding the unsafe usage of eval

* Add warning regarding the unsafe usage of exec

* Move warning under parameters table

* Use suggested shorter text

Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>

* Use suggested shorter text

Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>

* Improve wording as suggested

---------

(cherry picked from commit 00e5ec0)

Co-authored-by: Daniel Ruf <daniel@daniel-ruf.de>
Co-authored-by: Kirill Podoprigora <kirill.bast9@mail.ru>
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
@miss-islington-app
Copy link

Sorry, @DanielRuf and @willingc, I could not cleanly backport this to 3.12 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 00e5ec0d35193c1665e5c0cfe5ef82eed270d0f4 3.12

@bedevere-app
Copy link

bedevere-app bot commented Oct 30, 2024

GH-126161 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Oct 30, 2024
@brianschubert
Copy link
Contributor

@willingc I can make the 3.12 backport if that would be helpful

@willingc
Copy link
Contributor

Thanks @brianschubert. Ping me if you run into any issues.

willingc pushed a commit that referenced this pull request Oct 30, 2024
@miss-islington @DanielRuf
@Eclips4 @JelleZijlstra
[3.13] gh-118633: Add warning regarding the unsafe usage of eval and …
343a7a6 
…exec (GH-118437) (#126161)

gh-118633: Add warning regarding the unsafe usage of eval and exec (GH-118437)

* Add warning regarding the unsafe usage of eval

* Add warning regarding the unsafe usage of exec

* Move warning under parameters table

* Use suggested shorter text



* Use suggested shorter text



* Improve wording as suggested

---------

(cherry picked from commit 00e5ec0)

Co-authored-by: Daniel Ruf <daniel@daniel-ruf.de>
Co-authored-by: Kirill Podoprigora <kirill.bast9@mail.ru>
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
brianschubert pushed a commit to brianschubert/cpython that referenced this pull request Oct 30, 2024
@DanielRuf @Eclips4
@JelleZijlstra @brianschubert
[3.12] pythongh-118633: Add warning regarding the unsafe usage of eva…
1d11b8e 
…l and exec (pythonGH-118437)

* Add warning regarding the unsafe usage of eval

* Add warning regarding the unsafe usage of exec

* Move warning under parameters table

* Use suggested shorter text

Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>

* Use suggested shorter text

Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>

* Improve wording as suggested

---------

(cherry picked from commit 00e5ec0)

Co-authored-by: Daniel Ruf <daniel@daniel-ruf.de>
Co-authored-by: Kirill Podoprigora <kirill.bast9@mail.ru>
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
@bedevere-app
Copy link

bedevere-app bot commented Oct 30, 2024

GH-126162 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 only security fixes label Oct 30, 2024
hauntsaninja pushed a commit that referenced this pull request Oct 30, 2024
@brianschubert @DanielRuf
@Eclips4 @JelleZijlstra
[3.12] gh-118633: Add warning regarding the unsafe usage of eval and …
b69bb1e 
…exec (GH-118437) (#126162)

(cherry picked from commit 00e5ec0)

Co-authored-by: Daniel Ruf <daniel@daniel-ruf.de>
Co-authored-by: Kirill Podoprigora <kirill.bast9@mail.ru>
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
@DanielRuf DanielRuf deleted the patch-1 branch November 3, 2024 15:51
picnixz pushed a commit to picnixz/cpython that referenced this pull request Dec 8, 2024
@DanielRuf @Eclips4
@JelleZijlstra @picnixz
pythongh-118633: Add warning regarding the unsafe usage of eval and e…
abea066 
…xec (pythonGH-118437)

* Add warning regarding the unsafe usage of eval

* Add warning regarding the unsafe usage of exec

* Move warning under parameters table

* Use suggested shorter text

Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>

* Use suggested shorter text

Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>

* Improve wording as suggested

---------

Co-authored-by: Kirill Podoprigora <kirill.bast9@mail.ru>
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
ebonnal pushed a commit to ebonnal/cpython that referenced this pull request Jan 12, 2025
@DanielRuf @Eclips4
@JelleZijlstra @ebonnal
pythongh-118633: Add warning regarding the unsafe usage of eval and e…
7f7a1bc 
…xec (pythonGH-118437)

* Add warning regarding the unsafe usage of eval

* Add warning regarding the unsafe usage of exec

* Move warning under parameters table

* Use suggested shorter text

Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>

* Use suggested shorter text

Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>

* Improve wording as suggested

---------

Co-authored-by: Kirill Podoprigora <kirill.bast9@mail.ru>
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JelleZijlstra JelleZijlstra JelleZijlstra left review comments

@hugovk hugovk hugovk left review comments

@eli-schwartz eli-schwartz eli-schwartz left review comments

@nineteendo nineteendo nineteendo left review comments

@Eclips4 Eclips4 Eclips4 left review comments

@rhettinger rhettinger rhettinger approved these changes

@willingc willingc willingc approved these changes

Assignees

@willingc willingc

Labels
docs Documentation in the Doc dir skip news type-security A security issue
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@DanielRuf @Eclips4 @JelleZijlstra @willingc @brianschubert @hugovk @rhettinger @eli-schwartz @nineteendo @serhiy-storchaka
Morty Proxy This is a proxified and sanitized view of the page, visit original site.