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

Update minimum pillow dependency #6402

Merged
merged 1 commit into from Jun 5, 2022

Conversation

jarrodmillman
Copy link
Contributor

@jarrodmillman jarrodmillman commented Jun 4, 2022

There are a number of security issues (many marked high or critical) with pillow<9.0.1:
https://github.com/scikit-image/scikit-image/security/dependabot

@jarrodmillman jarrodmillman added this to the 0.20 milestone Jun 4, 2022
@hmaarrfk
Copy link
Member

hmaarrfk commented Jun 4, 2022

I don't know if we should be enforcing this at the installation level. Pillow 9.0.1 is pretty bleeding edge.

While there may be security vulnerabilities, it may be that certain users are not using those specific functions and thus it won't affect them.

9.0.1 was released Feb 2021 https://pypi.org/project/Pillow/9.0.1/

@stefanv
Copy link
Member

stefanv commented Jun 4, 2022

That's almost a year and a half ago, hardly bleeding edge.

@hmaarrfk
Copy link
Member

hmaarrfk commented Jun 4, 2022

i meant 2022. sorry

@jarrodmillman
Copy link
Contributor Author

jarrodmillman commented Jun 4, 2022

If for some reason, someone needs to use an old version of pillow that has several critical security issues, then they can.

pip install scikit-image
pip install pillow==<some old version with several known critical security issues>

Alternatively, they could use an older version of scikit-image. Since they aren't able to update the version of pillow they have installed, asking them to use an older version of scikit-image seems more than reasonable.

@hmaarrfk
Copy link
Member

hmaarrfk commented Jun 4, 2022

the thing is that these kinda of vulnerabilities will always occur. The only solution, is likely to be "most updated all the time".

However, for whatever reason, people choose to not update. Are you suggesting we force them to update in the spirit of security?

Users that update their environments will naturally be immune to these Security issues without us having to churn through updates on a regular basis.

@jarrodmillman
Copy link
Contributor Author

jarrodmillman commented Jun 4, 2022

Yes, I would recommend that you depend on things that don't have known critical security vulnerabilities. That will require staying on top of things because, as you say, new security issues are continually discovered.

Of course, for people like me who are worried about security, we will already have updated things. So this practice would mainly be to help users who aren't tracking security vulnerabilities.

I understand the points you are making. And I am assuming that you understand the issue that I am raising. I suspect we are coming to different conclusions based on the same information. Perhaps, the core developer team needs to decide on a security vs supporting old releases policy.

@hmaarrfk
Copy link
Member

hmaarrfk commented Jun 4, 2022

you are right. I think we both know the reasons for and against. I won't block this PR, but I do warn that it makes things harder for downstream users.

@stefanv
Copy link
Member

stefanv commented Jun 5, 2022

Thanks, Mark.

@stefanv stefanv merged commit 1ede871 into scikit-image:main Jun 5, 2022
@lagru lagru added the 🔧 type: Maintenance Refactoring and maintenance of internals label Oct 21, 2022
@jarrodmillman jarrodmillman deleted the security-pillow branch May 18, 2023 02:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔧 type: Maintenance Refactoring and maintenance of internals
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants