Skip to content
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.

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

Security fixes for 8.2.0 #5377

Merged
merged 10 commits into from Apr 1, 2021
Merged

Conversation

@hugovk
Copy link
Member

@hugovk hugovk commented Apr 1, 2021

See commits and release notes for details.

(Plus reorder release notes to roughly alphabetical, nothing requiring promoting to the top.)

hugovk and others added 9 commits Apr 1, 2021
* For J2k images with multiple bands, it's legal in to have different
  widths for each band, e.g. 1 byte for L, 4 bytes for A
* This dates to Pillow 2.4.0
* The readline used in EPS has to deal with any combination of \r and
  \n as line endings. It used an accidentally quadratic method of
  accumulating lines while looking for a line ending.
* A malicious EPS file could use this to perform a DOS of Pillow in
  the open phase, before an image was accepted for opening.
* This dates to the PIL Fork
* FliDecode did not properly check that the block advance was
  non-zero, potentally leading to an infinite loop on load.
* This dates to the PIL Fork
* Found with oss-fuzz
* A corrupt or specially crafted TTF font could have font metrics that
  lead to unreasonably large sizes when rendering text in
  font. ImageFont.py did not check the image size before allocating
  memory for it.
* Found with oss-fuzz
* This dates from the PIL fork
* PSDImagePlugin did not sanity check the number of input layers and
  vs the size of the data block, this could lead to a DOS on
  Image.open prior to Image.load.
* This issue dates to the PIL fork
* BlpImagePlugin did not properly check that reads after jumping to
  file offsets returned data. This could lead to a DOS where the
  decoder could be run a large number of times on empty data
* This dates to Pillow 5.1.0
@hugovk hugovk added this to the 8.2.0 milestone Apr 1, 2021
@hugovk
Copy link
Member Author

@hugovk hugovk commented Apr 1, 2021

@wiredfool There are 3 fails (in addition to xfails) on the valgrind workflow, is this good to merge or do we want to check those first?

src/libImaging/Jpeg2KDecode.c Outdated Show resolved Hide resolved
@wiredfool
Copy link
Member

@wiredfool wiredfool commented Apr 1, 2021

I know these patches are good, and I'm not too worried because they're 3 separate images in the fuzz_images test, which is mainly ensuring that the fuzzer runs under test (but not as fuzzing).

@hugovk
Copy link
Member Author

@hugovk hugovk commented Apr 1, 2021

Thanks, proceeding with the release!

Co-authored-by: Andrew Murray <3112309+radarhere@users.noreply.github.com>
@hugovk hugovk merged commit ee635be into python-pillow:master Apr 1, 2021
1 check was pending
@hugovk hugovk deleted the security-and-release-notes branch Apr 1, 2021
@guitos
Copy link

@guitos guitos commented May 6, 2021

Hi, the CVEs mentioned in commits and release notes will be reported to Mitre?
Thanks.

@radarhere
Copy link
Member

@radarhere radarhere commented May 6, 2021

They have been, yes. Go to https://pillow.readthedocs.io/en/stable/releasenotes/8.2.0.html#security to see links to the reports.

@tcullum-rh
Copy link

@tcullum-rh tcullum-rh commented May 6, 2021

Why are there 2 CVEs assigned for "Fix OOB read in Jpeg2KDecode?" Could someone please explain? @hugovk @radarhere @wiredfool

@hugovk
Copy link
Member Author

@hugovk hugovk commented May 6, 2021

One each for:

  • CVE-2021-25287: "There is an out-of-bounds read in J2kDecode, in j2ku_graya_la."

  • CVE-2021-25288: "There is an out-of-bounds read in J2kDecode, in j2ku_gray_i."


The CVEs from https://pillow.readthedocs.io/en/stable/releasenotes/8.2.0.html#security at MITRE (example) are still showing as reserved:

** RESERVED ** This candidate has been reserved by an organization or individual that will use it when announcing a new security problem. When the candidate has been publicized, the details for this candidate will be provided.

Do we or Tidelift need to do anything to publicise them?

@radarhere
Copy link
Member

@radarhere radarhere commented May 6, 2021

https://cve.mitre.org/cve/researcher_reservation_guidelines#researcher_reservation_guidelines#12

  1. Notify the CVE Team that the vulnerability has been made public using the CVE Request web form, and selecting "Notify CVE about a Publication."

I've submitted https://cveform.mitre.org/ accordingly.

@wiredfool
Copy link
Member

@wiredfool wiredfool commented May 7, 2021

There were 2 cves assigned for 2 fuzzier test cases that looked very similar before I got to fixing it. Once the fix was done, there was one bug that could hit any multiband mode but the fuzzier only found these two.

@radarhere
Copy link
Member

@radarhere radarhere commented Jun 4, 2021

I received an e-mail response, and the CVEs should now be public.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants