search

google / osv.dev

Open source vulnerability DB and triage service.
https://osv.dev
Apache License 2.0
1.55k stars 190 forks source link

Crucial bug: osv-scanner does not detect known malicious package in lockfiles #2407

Open maaaaz opened 4 months ago

maaaaz commented 4 months ago

Hello there,

Thanks for this amazing work but I am reporting here a crucial bug: known malicious packages are not detected when scanned.

How to reproduce:

Nothing is told about this pymocks package.

I tried with different expressions: pymocks==0.0.1, pymocks etc. but it never got detected. As this package is globally malicious, its detection should not need a version string: the sole presence of the package name in a lockfile should be enough to detect it !

Cheers!

G-Rath commented 4 months ago

This looks like an osv.dev API bug, as both osv-detector and osv-scanner --experimental-offline report this vulnerability

andrewpollock commented 4 months ago

I can confirm that

$ curl -d \
          '{"version": "0.0.1", "package": {"name": "pymocks", "ecosystem": "PyPI"}}' \
          "https://api.osv.dev/v1/query"
{}

is not matching https://api.osv.dev/v1/vulns/MAL-2022-7426 so this is an OSV.dev API bug not an OSV-Scanner one. I'll move this over.

andrewpollock commented 4 months ago

Looking at https://api.osv.dev/v1/vulns/MAL-2022-7426, I can see what the problem is, and it's somewhat systemic to malicious packages records: because typically such packages get removed from the package registry, there are no versions to enumerate. OSV.dev's API today is reliant upon all known vulnerable (in this case, "malicious") versions being enumerated and present in the affected.versions[] field for detection.

This deficiency is being worked on in #2401

G-Rath commented 4 months ago

@andrewpollock would it be straightforward and worthwhile to introduce a (hopefully) hotpath for advisories that are marked as impacting all versions, since shouldn't that be a case of matching the ecosystem + name?

I'd say that technically it's an optimization which as a bonus would enable these advisories to be matched against without requiring a more fulsome api change

maaaaz commented 4 months ago

Hello and thanks for taking care of that issue.

would it be straightforward and worthwhile to introduce a (hopefully) hotpath for advisories that are marked as impacting all versions, since shouldn't that be a case of matching the ecosystem + name?

I totally agree, version is irrelevant for known malicious packages.

another-rex commented 4 months ago

There are cases where version do matter for malicious packages, e.g. in cases where a normal package repository was taken over by a malicious actor, and they made a new release containing malicious code. All previous versions are still valid non-malicious packages in that case (or sometimes not, if the registry is not immutable, the attacker might be able to swap out old versions as well).

maaaaz commented 4 months ago

I don't know if and how this issue is linked to this one: https://github.com/github/advisory-database/issues/4612

andrewpollock commented 4 months ago

I don't know if and how this issue is linked to this one: github/advisory-database#4612

They're related only in that they both relate to malware (as opposed to security vulnerabilities)

github-actions[bot] commented 2 months ago

This issue has not had any activity for 60 days and will be automatically closed in two weeks

See https://github.com/google/osv.dev/blob/master/CONTRIBUTING.md for how to contribute a PR if you're interested in helping out.

another-rex commented 2 months ago

Looking at this again, I think a solution if a package is removed (therefore cannot be enumerated) is to assume all versions are vulnerable?

If it was the case I commented above, and the original maintainer regains control, I would guess that the entire package would not be removed from the repository, but only the latest version will be yanked.

We would need a way to determine if us not being able to enumerate is not a temporary/transient error, or because a package is removed completely.

maaaaz commented 2 months ago

if a package is removed (therefore cannot be enumerated) is to assume all versions are vulnerable?

I agree.

When a non-legitimate-package-spoofing malicious package is found, removed or not, it should be simply marked as malicious by the package name.

github-actions[bot] commented 4 days ago

This issue has not had any activity for 60 days and will be automatically closed in two weeks

See https://github.com/google/osv.dev/blob/master/CONTRIBUTING.md for how to contribute a PR if you're interested in helping out.