FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Forbid Lambda expressions in ImageMath.eval() by feliperalmeida · Pull Request #5963 · python-pillow/Pillow · GitHub

/ Pillow Public

Forbid Lambda expressions in ImageMath.eval() - #5963

Closed
feliperalmeida wants to merge 1 commit into
python-pillow:mainfrom
feliperalmeida:imagemath_eval_anon
ConversationCommits (1)ChecksFiles changed
Closed

Forbid Lambda expressions in ImageMath.eval()#5963
feliperalmeida wants to merge 1 commit into
python-pillow:mainfrom
feliperalmeida:imagemath_eval_anon

Conversation

feliperalmeida commented Jan 16, 2022 •
edited
Loading

Copy link
Copy Markdown

Description

This is an additional fix for CVE-2022-22817 (addressed in #5923). As commented in the original PR, it's still possible to execute arbitrary code via lambda expressions - e.g.: ImageMath.eval("(lambda: exit())()").

Original Fix

The original fix checks all co_names in the compiled expression passed to ImageMath.eval() against builtins names. However, lambda expressions generates anonymous functions and they're not listed in the co_names structure.

Changes in this PR

This PR forbids Lambda expressions being passed to ImageMath.eval() by verifying all functions names in the literals section of the bytecode (co_consts).

Trade-offs

Lambda expressions will no longer be supported in ImageMath.eval(). If such support is desired, all code objects inside co_consts can be verified in the same way as the original fix does (iterating through co_names and verifying against builtins names). Please let me know in the comments I can rewrite this fix.

radarhere commented Feb 2, 2022

Copy link
Copy Markdown
Member

Hi. I've just merged #6009 instead of this.
Please don't take that to mean that we completely ignored your PR - it's just that this is a CVE, so it seemed better to not draw too much attention to it by discussing it openly. We considered this privately, and went with #6009, so that users can still use lambdas if they want. That fix will be shortly be part of Pillow 9.0.1.
If you have any further thoughts, let us know.

hugovk commented Feb 3, 2022

Copy link
Copy Markdown
Member

@feliperalmeida And thanks for the PR!

In the future, when it comes to security-related issues, please could you check and follow the security policy of the project? If there's none available, it's good practice to ask how to disclose.

The Pillow one is here:

https://github.com/python-pillow/Pillow/security/policy

Thanks again!

feliperalmeida commented Feb 3, 2022

Copy link
Copy Markdown
Author

@hugovk Sure, no problem. The only reason I opened the PR instead of communicating privately was because it was already made "public" in this comment. So I thought raising a PR would help to expedite the fix.

Thanks.

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL