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

NPM packages with namespaces from ReversingLabs don't have namespace in name. #555

Closed
calebbrown opened this issue Jun 28, 2024 · 10 comments · Fixed by #559 or #667
Closed

NPM packages with namespaces from ReversingLabs don't have namespace in name. #555

calebbrown opened this issue Jun 28, 2024 · 10 comments · Fixed by #559 or #667
Assignees

Comments

@calebbrown
Copy link
Contributor

As at the time of reporting the number is 232 in NPM.

For example @zettle-bo/react is listed as react.

@calebbrown
Copy link
Contributor Author

I will pause the reversing labs ingestion to ensure no new data is ingested until we have confirmed a fix.

Errant reports can either be corrected (moved to the correct location) in the case that the report doesn't yet exist, or withdrawn if the do.

calebbrown added a commit that referenced this issue Jun 28, 2024
This is to mitigate #555

Signed-off-by: Caleb Brown <[email protected]>
calebbrown added a commit that referenced this issue Jun 28, 2024
spencerschrock pushed a commit that referenced this issue Jun 28, 2024
* Add ability to pause/disable sources at config.

This is to mitigate #555

Signed-off-by: Caleb Brown <[email protected]>

* Pause reversing labs while #555 is resolved.

Signed-off-by: Caleb Brown <[email protected]>

* Disable gocognit linter for now.

Signed-off-by: Caleb Brown <[email protected]>

---------

Signed-off-by: Caleb Brown <[email protected]>
@calebbrown calebbrown self-assigned this Jun 28, 2024
@calebbrown
Copy link
Contributor Author

@rhalar

calebbrown added a commit that referenced this issue Jul 1, 2024
This helps prevent and detect cases like #555.

Signed-off-by: Caleb Brown <[email protected]>
@G-Rath
Copy link

G-Rath commented Jul 1, 2024

fwiw it looks like https://osv.dev/vulnerability/MAL-2024-3239 is also a result of this which is a version that actually overlaps with a valid used version - it would be good to know how we can get these invalid advisories cleared since they're being flagged by the scanner.

I also wonder if osv.dev should be doing some validation around the PURLs as it sounds like they're invalid so maybe the affected field or the whole advisory be rejected rather than ingested?

calebbrown added a commit that referenced this issue Jul 1, 2024
calebbrown added a commit that referenced this issue Jul 1, 2024
Remove source data so the broken reports can be ingested again.

Signed-off-by: Caleb Brown <[email protected]>
calebbrown added a commit that referenced this issue Jul 1, 2024
* Repair reports that incorrectly merged with existing reports.

Signed-off-by: Caleb Brown <[email protected]>

* Withdraw npm packages "react" and "shared" caused by #555.

Remove source data so the broken reports can be ingested again.

Signed-off-by: Caleb Brown <[email protected]>

* Repair reports that have multiple RL sources.

Signed-off-by: Caleb Brown <[email protected]>

* Repair novel single source reports.

Signed-off-by: Caleb Brown <[email protected]>

* Withdraw reports placed in the wrong location when another already exists.

Signed-off-by: Caleb Brown <[email protected]>

* Withdraw reports placed in the wrong location when another already exists.

Signed-off-by: Caleb Brown <[email protected]>

* Clean up two top level packages from the Reversing Labs data.

Signed-off-by: Caleb Brown <[email protected]>

* Clean up overlapping sources from Reversing Labs

Signed-off-by: Caleb Brown <[email protected]>

* Update the names to match the purls.

Signed-off-by: Caleb Brown <[email protected]>

---------

Signed-off-by: Caleb Brown <[email protected]>
@calebbrown calebbrown reopened this Jul 1, 2024
@calebbrown
Copy link
Contributor Author

False positives and data problems have been resolved. Some Reversing Labs reports need to be re-added, but that is less urgent.

calebbrown added a commit that referenced this issue Jul 1, 2024
This helps prevent and detect cases like #555.

Signed-off-by: Caleb Brown <[email protected]>
@rhalar
Copy link

rhalar commented Jul 1, 2024

Hey,

to clarify for anyone affected or following this issue, I work for ReversingLabs and am 'in charge' for exposing our advisories, and as such responsible for this mess :)
I contacted Caleb and we will resolve the source of the problem very soon, to avoid any further problems with the data we expose.

I'd like to apologize for any alarms this may have triggered, it was a dumb oversight we should have caught, and I take the full blame for it.
A huge shutout and thanks to @calebbrown for doing damage control and reacting so quickly.

Our security researchers work hard and do a great job at finding malicious packages across multiple ecosystems, I hate to see their work besmirched by what is essentially a conversion error by my side; we'll make sure this doesn't happen again.

It's not the best first impression but we are very excited by this collaboration, and hope to continually contribute to the OSSF going forward.

calebbrown added a commit that referenced this issue Jul 2, 2024
This complets the data restoration for #555.

Signed-off-by: Caleb Brown <[email protected]>
calebbrown added a commit that referenced this issue Jul 2, 2024
This complets the data restoration for #555.

Signed-off-by: Caleb Brown <[email protected]>
@calebbrown
Copy link
Contributor Author

The data fixing is now complete. I will close this issue once I have un-paused the reversing-labs source in the config.

@lujunsan
Copy link
Contributor

lujunsan commented Jul 8, 2024

Hi @calebbrown . It looks like some packages are missing from the "withdrawn" list, such as NPM qs (osv) or NPM angular (osv). Both of these were reverted because of this issue but they were not added to the withdrawn directory, which is causing some inconsistencies in our systems.

@calebbrown
Copy link
Contributor Author

@lujunsan I have fixed the summaries as well. Those reports are now clearly for @ozon-shared-deps/qs and @maia-web/angular.

Is your issue that the OSV files now point to different packages, despite previously pointing at angular and qs?

@lujunsan
Copy link
Contributor

Hi @calebbrown . I was under the impression that these packages that were at some point marked as malicious but later fixed should also be added to the "withdrawn" list, same as what was done for react: https://github.com/ossf/malicious-packages/blob/main/osv/withdrawn/npm/react/MAL-2024-2929.json

Is this not accurate? Like you say, the reports for those packages are correct now (and thanks for regenerating the summaries!), nothing to change there; I was just expecting to also find those fixed packages with entries in the withdrawn list, but I may have misunderstood the process.

@calebbrown
Copy link
Contributor Author

Hi @lujunsan, the problem that was corrected was that the purl field differed to the name of the package for NPM packages with a namespace.

Since it was ambiguous which package was being reported as malicious, for packages which did not have an existing report, it was decided to fix the ambiguity and move the report into the correct path, rather than withdrawing the report.

This means the MAL-* IDs are now pointing to the correct package so it is not possible to add a withdrawn report for the same IDs.

As far as I'm aware (and I plan to confirm this with the OSV team) OSV reports are designed to be mutable so they can be extended and corrected as needed. This should mean that this type of change should be okay to make.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment