Skip to content

Conversation

DanielRuf
Copy link
Contributor

@DanielRuf DanielRuf commented Apr 30, 2024

@ghost
Copy link

ghost commented Apr 30, 2024

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

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.

@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
@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

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
DanielRuf and others added 2 commits May 7, 2024 08:41
Co-authored-by: Jelle Zijlstra <[email protected]>
Co-authored-by: Jelle Zijlstra <[email protected]>
@serhiy-storchaka serhiy-storchaka added the needs backport to 3.13 bugs and security fixes label May 9, 2024
@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
4 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
…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 <[email protected]>

* Use suggested shorter text

Co-authored-by: Jelle Zijlstra <[email protected]>

* Improve wording as suggested

---------

(cherry picked from commit 00e5ec0)

Co-authored-by: Daniel Ruf <[email protected]>
Co-authored-by: Kirill Podoprigora <[email protected]>
Co-authored-by: Jelle Zijlstra <[email protected]>
@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
Member

@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
…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 <[email protected]>
Co-authored-by: Kirill Podoprigora <[email protected]>
Co-authored-by: Jelle Zijlstra <[email protected]>
brianschubert pushed a commit to brianschubert/cpython that referenced this pull request Oct 30, 2024
…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 <[email protected]>

* Use suggested shorter text

Co-authored-by: Jelle Zijlstra <[email protected]>

* Improve wording as suggested

---------

(cherry picked from commit 00e5ec0)

Co-authored-by: Daniel Ruf <[email protected]>
Co-authored-by: Kirill Podoprigora <[email protected]>
Co-authored-by: Jelle Zijlstra <[email protected]>
@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
…exec (GH-118437) (#126162)

(cherry picked from commit 00e5ec0)

Co-authored-by: Daniel Ruf <[email protected]>
Co-authored-by: Kirill Podoprigora <[email protected]>
Co-authored-by: Jelle Zijlstra <[email protected]>
@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
…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 <[email protected]>

* Use suggested shorter text

Co-authored-by: Jelle Zijlstra <[email protected]>

* Improve wording as suggested

---------

Co-authored-by: Kirill Podoprigora <[email protected]>
Co-authored-by: Jelle Zijlstra <[email protected]>
ebonnal pushed a commit to ebonnal/cpython that referenced this pull request Jan 12, 2025
…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 <[email protected]>

* Use suggested shorter text

Co-authored-by: Jelle Zijlstra <[email protected]>

* Improve wording as suggested

---------

Co-authored-by: Kirill Podoprigora <[email protected]>
Co-authored-by: Jelle Zijlstra <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir skip news type-security A security issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants