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

Antivirus Avast detect test image crash_2.tif as with exploit #4730

Closed
danieltomasz opened this issue Jun 26, 2020 · 15 comments · Fixed by #4929
Closed

Antivirus Avast detect test image crash_2.tif as with exploit #4730

danieltomasz opened this issue Jun 26, 2020 · 15 comments · Fixed by #4929
Labels

Comments

@danieltomasz
Copy link

danieltomasz commented Jun 26, 2020

What did you do?

My Antivirus (Avast Shield alert) keep reporting and removing file crash_2.tif to my virus chest because it has vulnerability with TIFF:CVE-2015-5097 [Expl] .
This image resides on path /private/var/folders/..../unpacked/Pillow-7.1.2/Tests/images/crash_2.tif

What did you expect to happen?

Any file distributed with Pillow will not be reported as an exploit.

What actually happened?

What are your OS, Python and Pillow versions?

  • OS: MacOS 10.15.5
  • Python: 3.8.2
  • Pillow: 7.1.2
code goes here
@wiredfool
Copy link
Member

They're not wrong, but it's not helpful. Anything that's named crash_ is a reproduction of a crashing bug in Pillow, most of them have CVEs.

@hugovk
Copy link
Member

hugovk commented Jun 26, 2020

And this one was specifically for CVE-2020-10379 "Fix two buffer overflows in TIFF decoding" (#4507):

@danieltomasz
Copy link
Author

They're not wrong, but it's not helpful. Anything that's named crash_ is a reproduction of a crashing bug in Pillow, most of them have CVEs.

I am not importing Pillow directly, it was installed as a dependency for other packages. If I could isolate more I will post it here.

@radarhere
Copy link
Member

It's a vulnerability for a past version of Pillow. It is not a vulnerability for Pillow 7.1.2 that you have.

@radarhere
Copy link
Member

Also, to be clear, the image file can be removed by your antivirus software and Pillow will continue to work without a problem. The image would only be used if you ran a specific Pillow test script.

@danieltomasz
Copy link
Author

If this is solved already, the issue could be closed ( I filled it to let others know about the issue) I removed the aforementioned file and could use Pillow without problems

@Zen-CODE
Copy link

I've also just hit this issue when building pillow for a Kivy python deployment on iOS. Got it whilst trying to update from version 6.2.0 to 7.20. And sorry, deleting the file after deployment is not a solution. It's a workaround.

Consider if someone is building a Kivy app on MacOS for the first time and they sees this Virus warning immediately when they build? It's a terrible impression, and they will probably cancel and uninstall everything. Please could you consider a proper solution, that avoids this warning? i.e. removing the files or updating with non-vulnerable ones?

Please refer: kivy/kivy-ios#529

@radarhere
Copy link
Member

My suggested solution would be to move the CVE images to a separate python-pillow repository, and to fetch an image as part of each individual test. I don't personally have permissions to create such a repository though.

@radarhere radarhere reopened this Jul 15, 2020
@hugovk
Copy link
Member

hugovk commented Jul 15, 2020

They could go in https://github.com/python-pillow/pillow-depends/tree/master/test_images

These are fetched on the CIs but aren't in the sdist:

But would this be a problem for downstream packagers, who usually want tests too? Would they be okay not testing for CVE regressions by default?

@radarhere
Copy link
Member

Thanks for providing that URL. I tried finding it instead at https://github.com/wiredfool/test-image-results, which isn't right.

I've created #4792 to clarify what I'm suggesting - download each image as part of each test, so that everyone would still be testing for CVE regressions, but without triggering antivirus software. I recognise this may not be an ideal solution, just putting the idea out there.

@Zen-CODE
Copy link

@radarhere @hugovk Thanks. I really appreciate the action on this. +1

@radarhere
Copy link
Member

#4792 (comment)

Tests shouldn't rely on the network being available -- The tests need to be deterministic based on what's on the disk.

Either:

  1. It should stay, it's important
  2. It should go in the additional test images repo
  3. It should be in a pw protected zip/xor'd file with a known password like 'sudo_ignore_this_avast_this_is_a_false_positive'

(fwiw, tests that rely on network have been a pain over the last months as I've been on connections that are flakey to one level or other, and cell hotspots aren't much better)

@radarhere
Copy link
Member

I've created PR #4869 to use the zip suggestion.

@hugovk
Copy link
Member

hugovk commented Sep 19, 2020

#4792 (comment)

Tests shouldn't rely on the network being available -- The tests need to be deterministic based on what's on the disk.
Either:

  1. It should stay, it's important
  2. It should go in the additional test images repo
  3. It should be in a pw protected zip/xor'd file with a known password like 'sudo_ignore_this_avast_this_is_a_false_positive'

(fwiw, tests that rely on network have been a pain over the last months as I've been on connections that are flakey to one level or other, and cell hotspots aren't much better)

I've a slight preference for number 2, just because it simplifies our test mechanics and requires less special handling code.

But if anyone prefers number 3 (PR #4869), that's fine too.

@radarhere
Copy link
Member

I've created #4929 for number 2.

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