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

Moved CVE images to pillow-depends #4929

Merged
merged 1 commit into from
Sep 22, 2020
Merged

Conversation

radarhere
Copy link
Member

@radarhere radarhere commented Sep 22, 2020

Resolves #4730. Alternative to #4869

The Pillow test suite contains several images to trigger past CVEs, for the purpose of ensuring that they do not recur. However, as the issue describes, antivirus software is not aware that Pillow is up-to-date and so these are no longer vulnerabilities.

This PR is part of moving those images to pillow-depends. python-pillow/pillow-depends#32 is the other part, and this PR fails without it.

This PR also changes the test so that it is run as part of the test suite, but skips the test if run locally without the test images.

@nulano
Copy link
Contributor

nulano commented Sep 22, 2020

Would it make sense to put the test images in a subdirectory to make them easier to remove from a checked-out copy? All future CVE examples could be put in there as well.

For example Tests/images/cve/crash_1.tif, similarly to

EXTRA_DIR = "Tests/images/picins"
YA_EXTRA_DIR = "Tests/images/msp"
@pytest.mark.skipif(
not os.path.exists(EXTRA_DIR), reason="Extra image files not installed"
)

@hugovk
Copy link
Member

hugovk commented Sep 22, 2020

Yeah, we could put them in a cve subdir.

Normally we can't disclose CVE details until the fix is released and announced, but I expect we'd first put them in the main repo, and only move them to the other repo after someone reports a problem with their virus scanner?

Or let's keep it this way, and use a cve subdir if the need actually arises to be able easily delete them. Most people don't have test_images from the other repo anyway.

@nulano
Copy link
Contributor

nulano commented Sep 22, 2020

Normally we can't disclose CVE details until the fix is released and announced, but I expect we'd first put them in the main repo, and only move them to the other repo after someone reports a problem with their virus scanner?

Or let's keep it this way, and use a cve subdir if the need actually arises to be able easily delete them. Most people don't have test_images from the other repo anyway.

Fair points, I suppose these can be moved into a subdir if/when someone reports another file flagged by a virus scanner.

@hugovk
Copy link
Member

hugovk commented Sep 22, 2020

python-pillow/pillow-depends#32 is merged, and CIs restarted here.

@hugovk hugovk merged commit eb00829 into python-pillow:master Sep 22, 2020
@hugovk
Copy link
Member

hugovk commented Sep 22, 2020

Thank you!

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

Successfully merging this pull request may close these issues.

Antivirus Avast detect test image crash_2.tif as with exploit
3 participants