[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Python review request, CVE-2022-22817 & CVE-2023-50447 in pillow



Hello,

I have three review requests for src:pillow in LTS and ELTS.

(1)  I believe that the fixes previously uploaded to buster, stretch and
jessie for CVE-2022-22817 are incomplete.  Upstream updated the
vulnerability a month after releasing the original fix with a follow-up
fix in commit c930be075 on their 9.0.x branch, which we never uploaded.

I was thinking that it would be appropriate to issue DLA-..-2 and
ELA-..-2 advisories, but the problem is that buster was under Security
Team support at the time of the previous update, and stretch was under
LTS, not ELTS.  Another option would be to roll the information into my
advisories for CVE-2023-50447, the fixes for which I'll upload at the
same time.  What would be preferable?

(2)  I discovered that the backporting of the test for the fix for
CVE-2022-22817 to stretch has a subtle problem with Python 2.
The upstream patch uses

    with pytest.raises(ValueError):
        ImageMath.eval("exec('pass')")

and our backport is

    self.assertRaises(Exception, lambda: ImageMath.eval("exec('pass')"))

On Python 2 this assertion succeeds, but the exception raised is a
SyntaxError, not the ValueError we want to see.  This is because exec is
a statement, and not a function, in Python 2.  In other words, the
SyntaxError results in a false positive passing of the test.

Emilio, do you recall whether this was a deliberate modification of the
test, or does my analysis seem correct?  I appreciate the ELA was some
time ago.  I haven't yet had a proper look at jessie or buster.

(3)  The new tests for the follow-up fix for CVE-2022-22817, and a new
test for CVE-2023-50447, have expressions like this:

    self.assertRaises(ValueError, lambda:
        ImageMath.eval("(lambda: exec('exit()'))()", {"exec": None}))

So there is a similar problem with exec changing from a statement to a
function from Python 2 to Python 3.  I tried conditionalising, but it
seems that "(lambda: exec 'exit()')()" is a syntax error on Python 2.

A way to address both the bogus SyntaxError problem described above and
this other problem is to use eval instead of exec for all the tests.
Based on my understanding of the vulnerability I think that this
modification to the tests is okay, but it would be best if someone with
more knowledge of Python's evaluation model thinks it through.

I will momentarily push my work for review to the debian/stretch branch
of salsa:lts-team/packages/pillow.
There is a nice description of the vulnerability here:
<https://duartecsantos.github.io/2023-01-02-CVE-2023-50447/>.

Thanks.

-- 
Sean Whitton

Attachment: signature.asc
Description: PGP signature


Reply to: