-
Notifications
You must be signed in to change notification settings - Fork 204
Fix error in true/false positive crash designation on the experiment result page #1055
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 error in true/false positive crash designation on the experiment result page #1055
Conversation
…true vulnerabilities are reported as true positives.
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
DonggeLiu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @AmPaschal for noticing and fixing this.
Some nits before merging.
report/templates/index.html
Outdated
| <td data-sort-value="{{bug_sample.benchmark.project}}">{{bug_sample.benchmark.project}}</td> | ||
| <td data-sort-value="{{bug_sample.benchmark.id}}"><a href="sample/{{ bug_sample.benchmark.id|urlencode }}/{{ bug_sample.sample.id }}.html">{{bug_sample.benchmark.id}}-{{ bug_sample.sample.id }} </a> </td> | ||
| <td style="color: {{ 'red' if bug_sample.sample.result.crashes and not bug_sample.sample.result.is_semantic_error else 'green' }}" data-sort-value="{{'True positive' if bug_sample.sample.result.is_semantic_error else 'False positive'}}">{{'True positive' if bug_sample.sample.result.is_semantic_error else 'False positive'}}</td> | ||
| <td style="color: {{ 'red' if bug_sample.sample.result.crashes and bug_sample.sample.result.is_semantic_error else 'green' }}" data-sort-value="{{'True positive' if not bug_sample.sample.result.is_semantic_error else 'False positive'}}">{{'True positive' if not bug_sample.sample.result.is_semantic_error else 'False positive'}}</td> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-
Shall we keep the color consistent with the existing design? I.e., red for true positive and green for false positive.
Essentially, true positive is more alarming because it indicates a potential vul should be reported to the project maintainers. -
Would it be clearer if we do:
'False positive' if bug_sample.sample.result.is_semantic_error else 'True positive'There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, these suggestions look good. I have changed the color and text accordingly.
DonggeLiu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
|
/gcbrun skip |
+1 Thanks for noticing this @AmPaschal ! Great work catching this. |
This PR fixes an error in the oss-fuzz-gen's experiment result page where true positive crashes are classified as false positives, and vice versa.
In report/templates/benchmark.html, crashes are designated as True vulnerabilities if
sample.result.crashes and **not** sample.result.is_semantic_error.However, in report/templates/index.html, the reverse occurs. Crashes were previously designated as True positives if
bug_sample.sample.result.is_semantic_error. This PR fixes this discrepancy.