Upgrading min Pillow requirement version to 6.1.0

[alexdesiqueira]

Description

Dependabot is showing some issues on Pillow < 8.1.1. Maybe upgrading won't hurt?

Checklist

• Docstrings for all functions
• Gallery example in ./doc/examples (new features only)
• Benchmark in ./benchmarks, if your changes aren't covered by an
  existing benchmark
• Unit tests
• Clean style in the spirit of PEP8

For reviewers

• Check that the PR title is short, concise, and will make sense 1 year
  later.
• Check that new functions are imported in corresponding __init__.py.
• Check that new features, API changes, and deprecations are mentioned in
  doc/release/release_dev.rst.

[alexdesiqueira]

According to the discussion in #5340, where @jni says:

... we use NEP-29 as a guide for how long to keep our minimal dependencies.

This way, we have:

❯ nep29 pillow
| version |    date    |
|---------|------------|
|  8.2.0  | 2021-04-01 |
|  8.1.0  | 2021-01-02 |
|  8.0.0  | 2020-10-14 |
|  7.2.0  | 2020-06-30 |
|  7.1.0  | 2020-04-01 |
|  7.0.0  | 2020-01-02 |
|  6.2.0  | 2019-10-01 |
|  6.1.0  | 2019-07-03 |

So we could update Pillow to 6.1.0 in July. On the other hand, Dependabot says:

Pillow before 8.1.1 allows attackers to cause a denial of service (memory consumption) because the reported size of a contained image is not properly checked for a BLP container, and thus an attempted memory allocation can be very large.

... and other scary warnings. For now, I'll leave this open for discussion, if you'd like. Else, please close it 🙂

[sciunto]

we mainly have PIL as a direct dependency for io collections, and indirectly from imageio. A way to attack this issue is to remove direct PIL imports. THis is something I investigated, with the help of @jni. It appears that we have a piece of code to load series of tiff images and series of animated gif images. Afaik, the main stopper is that imageio doesn't have the capability to read animated gif and we delegate this to PIL. I don't know how much reading animated gif is important.

I believe this point of view can help in the debate as well.

[jni]

Yeah, I've always considered security "above my pay grade". This all just makes me think I was right. 😂 And also about delegating all io to imageio.

At any rate: I think the NEP-29 guideline is just that — a guideline, and it can be overridden in cases where it makes sense. If pillow 8.1.2 is available on conda-forge, Anaconda defaults, etc, I'm happy to bump up the dependency.

Another alternative is to defer our imports of pillow and raise a warning when it is imported if the version predates 8.1.2.

I'm ok with either option, so I'm going to approve this and let the next person decide. ;)

[sciunto]

In the past, we decided to set a warning.

[alexdesiqueira]

Another alternative is to defer our imports of pillow and raise a warning when it is imported if the version predates 8.1.2.

@sciunto:

In the past, we decided to set a warning.

How would be that warning?

[sciunto]

See #4861 ;)

[alexdesiqueira]

I did some research today on this:

Afaik, the main stopper is that imageio doesn't have the capability to read animated gif and we delegate this to PIL.

It seems that imageio does this now — through PIL.

A way to attack this issue is to remove direct PIL imports. THis is something I investigated, with the help of @jni. It appears that we have a piece of code to load series of tiff images and series of animated gif images.

Maybe we could use imageio's alternative; they would deal with Pillow, then. How does that sound?

[sciunto]

THen, I guess you want to relax the min version

[alexdesiqueira]

see above for the min req
THen, I guess you want to relax the min version

Sorry @sciunto, I didn't get it 🙂

[sciunto]

Don't you want to set >=6.1.0 or so? otherwise, the warning doesn't make sense anymore.

[alexdesiqueira]

I expect it to be >=8.1.2, but I updated the CVE numbers on the warning to reflect the latest issues. Isn't it the way to do here?
What would you prefer? To set up >=6.1.0?

[sciunto]

If you choose > 8.1.2, no need to have the warning, because the users will be forced to upgrade. If we do not want to force the upgrade, then warning and >6.1.0. THat's my understanding.

[alexdesiqueira]

@sciunto got it. I thought the warning would check if the machine has that version, and would ask to update based on that.
Warning off, then? 🙂

[sciunto]

Maybe we could use imageio's alternative; they would deal with Pillow, then. How does that sound?

Yes, I was reading imageio's doc and didn't see that support. But, if it works, then definitely yes.

[sciunto]

see above for the min req

[sciunto]

Maybe we could use imageio's alternative; they would deal with Pillow, then. How does that sound?

Yes, I was reading imageio's doc and didn't see that support. But, if it works, then definitely yes.

We can try this in a different PR.

[hmaarrfk]

I feel like security issues are for system integrators to worry about. Problems and bugs come and go (well they come and they come).

security issues would bump us to bleeding edge.

[alexdesiqueira]

@hmaarrfk you have a point. I'll put 6.1.0 in this one, then, and leave the latest warnings; this way we'll get a full version up, at least

[sciunto]

Thank you @alexdesiqueira