-
Notifications
You must be signed in to change notification settings - Fork 188
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
fix: update the OSS-Fuzz denylist #2222
Conversation
Thanks for the change. Is there a list of false positive OSV entries you can point to as justification for this case? |
google/oss-fuzz#7434, google/oss-fuzz-vulns#25, google/oss-fuzz-vulns#35. At some point I stopped keeping track of them. I'd also point out things like https://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2022q1/016161.html
|
Thanks for the examples! Given the brittleness of MSan, I'm inclined for us to just start stop importing MSan issues by default period. This looks like some infrastructure issue that should've hopefully been fixed since. This looks like a legitimate OSS-Fuzz environment issue.
This wasn't us :) And we still have no idea who was submitting these CVEs.. particularly during an early time when this pipeline wasn't very mature. More generally, if we a) stop doing this automatically for all MSan issues, and b) improve infrastructure reliably to a point where you no longer see missing fixes, and implement the UX fixes in #2176 (comment), would you be open to turning this back on again? It's important for us to not turn this off by default for all OSS-Fuzz projects, because we've more often than not observed projects fixing vulnerabilities without ever notifying downstream consumers or requesting a CVE (because it's a lot of work). |
I think it depends. It would still be necessary to filter out reliability bugs that have nothing to do with security to prevent them from ending up in the OSV database and I'm not sure who is going to do that. Though if
was implemented I think they would be less likely to trigger overzealous vulnerability scanners.
I understand that but I'm not sure how it can be fixed. Downstream consumers should be able to deal with that in general. |
I'm thinking this can be a bit somewhere inside the |
As far as I understand it would work like "confidence" from #2176 (comment) so I think it should help. Though I suspect this bit is likely to be false in a lot of entries because (most?) projects aren't particularly interested in vetting OSV entries. |
/gcbrun |
This pull request has not had any activity for 60 days and will be automatically closed in two weeks |
Looks like the linter isn't happy. I'll reformat it and force-push the commit. |
2136023
to
57e1c76
Compare
I've just force-pushed the commit. The linter should be happy now hopefully. I was going to add more projects but decided not to partly because they are unlikely to attract that sort of attention and partly because I'm kind of hoping some sort of automated solution to this issue should pop up eventually and it would be nice if it could learn from false positives too. |
It's a follow-up to google#2201. The comments point to google#2178 because the reason is the same.
57e1c76
to
ac9c814
Compare
Looks like another linter isn't happy:
I'm not exactly sure what it should be. I'll add "fix" because it's a (temporary) fix from my perspective but if that title is wrong it can be changed by someone else. |
/gcbrun |
It's a follow-up to #2201. The comments point to #2178 because the reason is the same.