-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
idea: treat a major coverage drop an issue! #11398
Comments
I opened #7190 so that CIFuzz could catch coverage drops as soon as possible but it kind of stalled. The only way to keep track of drops like that I'm aware of is the Fuzz-Introspector dashboard but it works when there are less than ten fuzz targets. With more than 40 fuzz targets it gets blurry. |
Could you clarify this? It works when there are 10 or more as well? FWIF it's visible that there was a sudden jump in Curl's coverage: https://introspector.oss-fuzz.com/project-profile?project=curl so I think the drop would have been easy to spot but it looks like it happened at a point before the app was started (April 20 2023) |
The global dashboard itself works with a lot of fuzz targets. The problem is that sometimes coverage drops affect certain fuzz targets and it's hard to spot them because they are hidden behind the global coverage where several fuzz targets can cover the same code a bit differently. Basically I have to go over specific coverage reports one by one. |
FYI this feature now exists: ossf/fuzz-introspector#2102 I would wait for first results but we could maybe close this issue then. @bagder @evverx to gauge if this should be implemented, would you want to be notified if there are issues detected for a fuzz target or is the project view enough? |
I think it depends. If the coverage goes down significantly I think it should be certainly reported but ideally thresholds and things like that should be configurable probably to avoid spurious notifications. The latest coverage drop was #12519 (comment) (where one fuzz target went down) and as far as I understand with ossf/fuzz-introspector#2102 it should be easier to spot things like that. |
So would you prefer to be notified by default, let's say if there is a 5% drop for a fuzzer target, or only after configuration for a threshold has been set? |
I think there are two (kind of) different use cases here. It's probably safe to say that all the projects should be notified by default when their total coverage goes down significantly. Drops affecting specific fuzz targets/code paths are important too (for example GHSA-5fhj-g3p3-pq9g) but it's not always obvious how to deal with them. Personally I'd look at the dashboards every now and then for the time being to see how it works first. |
Thank you for the input! So to put this into actionable points:
What do you think? |
@phi-go I'm a bit reluctant when it comes to notifications because I'm subscribed to a bunch of different projects with different levels of tolerance to false positives so personally I think before adding opt-out notifications it would be great to figure out what it should do when fuzz targets are renamed, new code is added and so on. I don't think those things should trigger notifications to, say, mailing lists with hundreds of people there. |
Yeah, that's why I'm asking. I would expect the reports to be sent to the issue tracker as is already done, are these monitored by large mailing lists? But yes, it is probably better to keep everything opt-in at the beginning, maybe forever. From my experience renaming fuzz targets does not happen often. This could maybe be detected by code matching, though, this seems a bit error prone and difficult to implement. Coverage drops due to adding code happens often (obviously). But is this truly a false positive if the fuzzing setup has reduced coverage due to added project code? In a perfect world harnessing should be updated to address new code so a notification seems warranted. Adding code could also break the fuzzer harness. Though, I'm not so eager on having notification when vendoring third-party code, this also regularly causes coverage drops. However, here we would need to separate project code and third-party code, which is likely not trivial, probably another thing that should be configurable. (See here for a bit more discussion: ossf/fuzz-introspector#2054) Another common reason for coverage changes commonly seen as fluctuations are that the coverage process does not run through cleanly (think OOM or timeout). Should this be reported to maintainers? |
It depends on the project. For example the elfutils project is notified via the devel mailing list.
Agreed. Though I renamed fuzz targets a few times and it would have been weird if OSS-Fuzz had filed bug reports and sent notifications.
I think it depends. If the whole project is a hypothetical JSON parser it should probably be safe to assume that the new code is somehow related to the parser and should be fuzzed. In "umbrella" projects it's not always clear-cut. With no context I think it should stick to reporting obvious regressions it can be 100% sure about.
I'll take a look. Thanks!
I think those things should be more visible but I'm not sure they should always lead to notifications. It seems to me dashboards should be a good place to experiment, collect project-specific data and act on those things. |
Yes, fair enough. What do you think of optionally allowing maintainers to specify a list of fuzz targets that should be fuzzed? We can use a heuristic based on fuzzers available in the past if this list is not specified. Renaming a fuzz target would still cause an alert in the later case. Also personally I would actually like to see that the fuzz targets have changed as the oss-fuzz interaction is pretty much async this would signal to me that my change worked but I guess having a dashboard the same can be confirmed.
While there is the difference that for a JSON parser the new code should be covered by the existing harness this is not necessarily the case for a more diverse project. However, in both cases new code is added that should be fuzzed, is that not a clear regression? Or do you mean that the new code might not need to be fuzzed? I'm probably missing something but without context how can we detect obvious regressions?
I agree, let's take it slow and extend the dashboard, we can still do notifications later. |
It can probably work if projects are actively involved in maintaining their fuzzing infrastructure but I don't think there are a lot of projects where it happens in practice. That's why I don't think that manual tweaks can work in practice in general. There are FI issues where it's discussed how maintainers can manually specify their threat models, exclude coverage and so on but it's unlikely a lot of projects are going to do that.
Agreed. That's why I think CIFuzz (which is run when PRs are opened) should do as much as possible to make sure stuff works/prevent regressions from landing. It's not there yet though.
I was trying to say that in some cases it's hard to tell whether code should be fuzzed at all. It's also worth mentioning that things like static reachability and so on aren't available everywhere because there are projects where FI just doesn't work.
As far as I understand this issue was prompted by a huge coverage drop in the curl project and I think in cases like this it's enough to warrant a bug report with almost no context. I think there are things that need looking into (like how to auto-close those issues when nobody reacts to them for a year and a lot of things have changed in the projects) but I think there should be a way for projects to say that they are interested in monitoring that metric. |
The curl code base has been fuzzed by oss-fuzz for many years. These days we rarely get any reports. It chugs away in the background and while it certainly gives us confidence and helps us "stay honest", it also falls off the top of our minds. We don't think about it too much in our daily work. I think that's good. It still tells us when it finds something. Which these days are many months between each time.
With this in mind, when we changed a config detail a year ago, the fuzz coverage for curl dropped considerably but no one noticed because no one checked. We are used to oss-fuzz telling us when there's an issue. Thus, oss-fuzz has the last year been fuzzing a much smaller part of the code base, which then of course could have made us miss problems.
The other day we fixed this issue and the coverage is again back up to levels similar to those a year ago, and nothing new has been reported (yet) so I figure in a way we were lucky and dodged one this time. But it brings up this topic on my agenda: how do I detect this problem the next time?
And perhaps even more broadly: how do we all (all oss-fuzz users) detect this because I presume that even though it happened to us this time, it might happen to others every once in a while. And maybe it has already happened to 22 users who have not even noticed yet and now the fuzzer is missing out?
How do others do? Is there a best practice I have missed? Can oss-fuzz perhaps itself detect huge drops and file an issue about that?
The text was updated successfully, but these errors were encountered: