X Tutup
Skip to content

Warn that overriding __builtins__ for eval is not a security mechanism#145773

Open
StanFromIreland wants to merge 5 commits intopython:mainfrom
StanFromIreland:eval_sec
Open

Warn that overriding __builtins__ for eval is not a security mechanism#145773
StanFromIreland wants to merge 5 commits intopython:mainfrom
StanFromIreland:eval_sec

Conversation

@StanFromIreland
Copy link
Member

@StanFromIreland StanFromIreland commented Mar 10, 2026

This misleads users as it gives the impression that the above warning can be ignored.


📚 Documentation preview 📚: https://cpython-previews--145773.org.readthedocs.build/

Copy link
Contributor

@sethmlarson sethmlarson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you!

@picnixz
Copy link
Member

picnixz commented Mar 10, 2026

I'm always torn when we change these kind of notes. I don't agree that this will lead to security vulnerabilities. It depends on your threat model. So the "may"/"can" formulation is for me the correct one.

What I do find correct however is that untrusted input can lead to security vulnerabilities. I would honestly drop the "easily" part as again it depends on where the untrusted input comes from. For instance, the user can supply an arbitrary string, and that arbitrary string is processed with (highly non-trivial, but still polynomial-time evaluable) reversible transformations that eventually output a string that is then evaled. In this case, you can consider that you have a security vulnerability, but not necessarily "easily" achievable (but still achievable if you spend time to reverse the transformations).

@nedbat
Copy link
Member

nedbat commented Mar 10, 2026

These warnings are aimed at educating people who don't understand the risks. I'm in favor of leaning more on the scary side. You are correct that the threat model matters, which is why I changed "will" to "can easily". I'd like to keep "easily" because we are trying to scare people away from these functions. The uninitiated will write code that can easily be security vulnerabilities.

You are also right that you can write code using these functions that are not vulnerable, and you can write code that is technically vulnerable but not easily vulnerable. We only have a sentence or two here, we need to use blunt language. We don't have space for the subtleties you are describing, and the people who need the warnings won't understand those subtleties.

@picnixz
Copy link
Member

picnixz commented Mar 10, 2026

In this case, I'm more in favor of a "can easily" rather than "will" (and thus agree with your suggestions). What I also do find valuable is the addition of "untrusted" term in the text.

StanFromIreland and others added 2 commits March 10, 2026 23:17
Co-authored-by: Ned Batchelder <ned@nedbatchelder.com>
@StanFromIreland StanFromIreland requested a review from picnixz March 10, 2026 23:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting merge docs Documentation in the Doc dir skip issue skip news type-security A security issue

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

5 participants

X Tutup