Skip to content

Strange security vulnerability 41002 #1198

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

Closed
sobolevn opened this issue Aug 2, 2021 · 6 comments
Closed

Strange security vulnerability 41002 #1198

sobolevn opened this issue Aug 2, 2021 · 6 comments
Labels
bug Something isn't working duplicate This issue or pull request already exists

Comments

@sobolevn
Copy link
Contributor

sobolevn commented Aug 2, 2021

Today all my builds (like literally all of them) failed, because coverage is reported to be insecure.
Full message:

 + safety check --full-report
+==============================================================================+
|                                                                              |
|                               /$$$$$$            /$$                         |
|                              /$$__  $$          | $$                         |
|           /$$$$$$$  /$$$$$$ | $$  \__//$$$$$$  /$$$$$$   /$$   /$$           |
|          /$$_____/ |____  $$| $$$$   /$$__  $$|_  $$_/  | $$  | $$           |
|         |  $$$$$$   /$$$$$$$| $$_/  | $$$$$$$$  | $$    | $$  | $$           |
|          \____  $$ /$$__  $$| $$    | $$_____/  | $$ /$$| $$  | $$           |
|          /$$$$$$$/|  $$$$$$$| $$    |  $$$$$$$  |  $$$$/|  $$$$$$$           |
|         |_______/  \_______/|__/     \_______/   \___/   \____  $$           |
|                                                          /$$  | $$           |
|                                                         |  $$$$$$/           |
|  by pyup.io                                              \______/            |
|                                                                              |
+==============================================================================+
| REPORT                                                                       |
| checked 156 packages, using free DB (updated once a month)                   |
+============================+===========+==========================+==========+
| package                    | installed | affected                 | ID       |
+============================+===========+==========================+==========+
| coverage                   | 5.5       | <6.0b1                   | 41002    |
+==============================================================================+
| Coverage 6.0b1 starts to use a modern hash algorithm (sha256) when           |
| fingerprinting for high-security environments.                               |
+==============================================================================+

Full logs example: https://github.com/wemake-services/wemake-django-template/pull/1667/checks?check_run_id=3216651270#step:7:1364

Why do I think that this is a strange security report?

  1. coverage is my dev tool, it don't think that this is an attack vector
  2. It is not clear how a hash algorithm can be a problem for calculating coverage while running tests on a trusted env
  3. It suggests to install a beta release, which is not the best practice at all

I might be wrong here, it might be a real problem, which is just not clear to me from the first sight.

Proposed solution

I see that I can report a security issue via Tidelift, but I don't see a security policy in the project.
Something like https://docs.github.com/en/code-security/getting-started/adding-a-security-policy-to-your-repository
Maybe it should be created? This way all huge user-base of your awesome project could benefit from understanding what is a security issue with coveragepy and what's not.

Related, npm has similar problems described here: https://overreacted.io/npm-audit-broken-by-design/

@sobolevn sobolevn added the bug Something isn't working label Aug 2, 2021
@nedbat
Copy link
Owner

nedbat commented Aug 2, 2021

Version 6.0b1 has already fixed this. The safety report you have even mentions the sentence from the changelog about it. It was reported in #1189.

@nedbat nedbat closed this as completed Aug 2, 2021
@nedbat nedbat added the duplicate This issue or pull request already exists label Aug 2, 2021
@bluenote10
Copy link

bluenote10 commented Aug 2, 2021

Since 6.0b1 is a beta release, pip doesn't use it by default. Would it make sense to a release a non-beta version? Technically there is currently no stable published version of coverage that passes the Safety scan.

@whyscream
Copy link

When reading #1189, it's not obvious to me that this is a security vulnerability, but rather a serious inconvenience/bug under some security-related circumstances. That would IMHO result in an incorrect listing in the safety database for this issue. @nedbat Could you shed some light at this?

@sobolevn
Copy link
Contributor Author

sobolevn commented Aug 2, 2021

@nedbat sorry, I think that I was not clear enough with my initial issue report.

Version 6.0b1 has already fixed this.

Version 6.0b1 fixes the #1189 bug, but my point is about coveragepy's security policy.

@nedbat
Copy link
Owner

nedbat commented Aug 2, 2021

@whyscream Right: this is not a security issue. It's a security scanner flagging any mention of md5. Coverage.py used md5 simply to fingerprint HTML files. There was no security concern. But it was easy to change to avoid these kinds of false alarms.

@whyscream
Copy link

@nedbat thanks, I created an issue for safety-db to remove the listing: pyupio/safety-db#2335. Let's see how that works out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working duplicate This issue or pull request already exists
Projects
None yet
Development

No branches or pull requests

4 participants