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

Document the properties of a high quality OSV record #2193

Merged
merged 43 commits into from
Jul 15, 2024

Conversation

andrewpollock
Copy link
Contributor

@andrewpollock andrewpollock commented May 9, 2024

Reference from existing documentation on data.

Part of #2186

@andrewpollock andrewpollock deleted the document_quality_bar branch May 23, 2024 04:50
@andrewpollock andrewpollock restored the document_quality_bar branch May 23, 2024 05:01
@andrewpollock andrewpollock reopened this May 23, 2024
docs/data_quality.md Outdated Show resolved Hide resolved
docs/data_quality.md Outdated Show resolved Hide resolved
Copy link
Contributor

@chrisbloom7 chrisbloom7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great doc! I left some very soft suggestions and nits along with a few grammatical fixes

docs/data_quality.md Outdated Show resolved Hide resolved
docs/data_quality.md Outdated Show resolved Hide resolved
docs/data_quality.md Outdated Show resolved Hide resolved
docs/data_quality.md Outdated Show resolved Hide resolved
docs/data_quality.md Outdated Show resolved Hide resolved
docs/data_quality.md Outdated Show resolved Hide resolved
docs/data_quality.md Outdated Show resolved Hide resolved
docs/data_quality.md Outdated Show resolved Hide resolved
docs/data_quality.md Outdated Show resolved Hide resolved
docs/data_quality.md Outdated Show resolved Hide resolved
andrewpollock and others added 15 commits June 3, 2024 10:51
Co-authored-by: Jon <darakian@github.com>
Feedback was that it was possible to perceive this as known unfixed
vulnerabilities as being of lower quality than ones with known/available
fixes, and that is not the case, as such known unfixed vulnerabilities
can and do legitimately exist (as is the case when the vulnerable
package is orphaned). These are a legitimate true positive with no
available fixed version.
Co-authored-by: Chris Bloom <chrisbloom7@github.com>
Co-authored-by: Chris Bloom <chrisbloom7@github.com>
Co-authored-by: Chris Bloom <chrisbloom7@github.com>
Co-authored-by: Chris Bloom <chrisbloom7@github.com>
Co-authored-by: Chris Bloom <chrisbloom7@github.com>
Co-authored-by: Chris Bloom <chrisbloom7@github.com>
Co-authored-by: Chris Bloom <chrisbloom7@github.com>
Co-authored-by: Chris Bloom <chrisbloom7@github.com>
Co-authored-by: Chris Bloom <chrisbloom7@github.com>
"Managing the Perishability of OSV Records" is in the early ideation
phase.
Copy link
Contributor

@chrisbloom7 chrisbloom7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

Copy link
Contributor

@darakian darakian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me 👍

andrewpollock and others added 17 commits June 6, 2024 15:44
Co-authored-by: Jon <darakian@github.com>
Feedback was that it was possible to perceive this as known unfixed
vulnerabilities as being of lower quality than ones with known/available
fixes, and that is not the case, as such known unfixed vulnerabilities
can and do legitimately exist (as is the case when the vulnerable
package is orphaned). These are a legitimate true positive with no
available fixed version.
Co-authored-by: Chris Bloom <chrisbloom7@github.com>
Co-authored-by: Chris Bloom <chrisbloom7@github.com>
Co-authored-by: Chris Bloom <chrisbloom7@github.com>
Co-authored-by: Chris Bloom <chrisbloom7@github.com>
Co-authored-by: Chris Bloom <chrisbloom7@github.com>
Co-authored-by: Chris Bloom <chrisbloom7@github.com>
Co-authored-by: Chris Bloom <chrisbloom7@github.com>
Co-authored-by: Chris Bloom <chrisbloom7@github.com>
Co-authored-by: Chris Bloom <chrisbloom7@github.com>
"Managing the Perishability of OSV Records" is in the early ideation
phase.
I realised I'd omitted the `events[]` array in the JSON path when
talking about `introduced` and `fixed`/`last_affected`.

Explicitly refer to commits *and* versions when discussing temporal
ordering of `introduced` and `fixed`/`last_affected`.
Copy link

@SecurityCRob SecurityCRob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the clarification in the documentation. I think this will be very useful to the OSV community & downstream consumers.

andrewpollock and others added 2 commits June 10, 2024 00:43
Based on feedback from Duy Truong about Android's patching approach.

## Rationale

OSV.dev seeks to be a comprehensive, accurate and timely database of known vulnerabilities that is highly automation friendly. In order to meet this accuracy goal, a quality bar needs to be both defined and sustainably enforced.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just out of curiosity given that it can't be sustainably enforced for OSV records coming from OSS-Fuzz (#2176) I wonder if they will no longer be imported (automatically)? If so I can probably close #2222.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll have to cross that bridge when we get to it, I think.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. As far as I understand it should be addressed in #2188 one way or another. Until then it would maybe make sense to mention that the quality bar isn't enforced yet. It should hopefully help to prevent treating all OSV entries as vulnerabilities. I'm 99% sure that bogus CVEs based on OSV data popped up because the OSV database was (and still is) advertised in a way that doesn't (always) match reality.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this discussion is separate to the intention of this document here.

This document is meant to serve as a reference for how to properly structure and format an OSV record. It doesn't go into whether or not a record is worthy of being considered a vulnerability. That would be a separate discussion :)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't go into whether or not a record is worthy of being considered a vulnerability

Currently the documentation says "It is also assumed that the vulnerability discussed in the OSV record is valid and affects the software described" and looking at #2193 (comment) I think it implies that OSV records are vulnerabilities.

Either way I agree that this discussion can be moved elsewhere. Just to clarify I think OSS-Fuzz is great and OSV is great. It's just that they don't interact well with each other yet and get misused from time to time by some organizations.

Copy link
Collaborator

@oliverchang oliverchang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with some minor nits.


## Rationale

OSV.dev seeks to be a comprehensive, accurate and timely database of known vulnerabilities that is highly automation friendly. In order to meet this accuracy goal, a quality bar needs to be both defined and sustainably enforced.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this discussion is separate to the intention of this document here.

This document is meant to serve as a reference for how to properly structure and format an OSV record. It doesn't go into whether or not a record is worthy of being considered a vulnerability. That would be a separate discussion :)

docs/data_quality.md Outdated Show resolved Hide resolved
docs/data_quality.md Outdated Show resolved Hide resolved
docs/data_quality.md Outdated Show resolved Hide resolved
docs/data_quality.md Outdated Show resolved Hide resolved
docs/data_quality.md Outdated Show resolved Hide resolved
docs/data_quality.md Outdated Show resolved Hide resolved
docs/data_quality.md Outdated Show resolved Hide resolved
@andrewpollock andrewpollock merged commit 8c2b48a into google:master Jul 15, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants