-
Notifications
You must be signed in to change notification settings - Fork 306
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
Export advisories in OSV format #599
base: master
Are you sure you want to change the base?
Conversation
e0825fa
to
e41fbe9
Compare
export-osv.php
Outdated
array_push($events, ['introduced' => $branch[0]]); | ||
array_push($events, ['fixed' => $branch[1]]); | ||
} else { | ||
array_push($events, ['fixed' => $branch[0]]); |
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.
Maybe we can just convert versions like <8.22.1
to 8.22.1
.
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.
How useful is OSV if we cannot list fixed versions at all?
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.
@naderman I worked around this by retrieving all package tags from the Packagist API and comparing the ranges with the tags by calling Semver::satisfies($version, implode(' ', $constraints))
.
Example: https://github.com/jaylinski/security-advisories/blob/osv/packagist/CVE-2021-43608.json
What do you think about it?
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.
Would it make more sense to add this to packagist.org's security advisories API as a new output format (which pulls data from this repo here)? As you'd have access to live package data there?
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, definitely. I wasn't aware of the packagist.org security advisories API.
Should I open a PR at the packagist.org repo?
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.
Sure, go ahead! :-)
It's pretty simple right now, just a way to list advisories for a set of packages, see https://packagist.org/apidoc#list-security-advisories
export-osv.php
Outdated
foreach (array_column($branches, 'versions') as $branch) { | ||
if (count($branch) === 2) { | ||
array_push($events, ['introduced' => $branch[0]]); | ||
array_push($events, ['fixed' => $branch[1]]); |
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.
be careful. We have cases like >= 1.3.0, <2.0
where things are not fixed in 2.0 (if another branch has >=2.0
in its affected constraints)
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.
That's true. I reworked it, see #599 (comment).
export-osv.php
Outdated
|
||
foreach (array_column($branches, 'versions') as $branch) { | ||
if (count($branch) === 2) { | ||
array_push($events, ['introduced' => $branch[0]]); |
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.
$branch[0]
is not a version but a constraint
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.
That's true. I reworked it, see #599 (comment).
export-osv.php
Outdated
] : null, | ||
[ | ||
'type' => 'PACKAGE', | ||
'url' => 'https://packagist.org/packages/' . $package, |
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.
this looks broken for packages using a custom repository
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.
I'm now skipping packages that have a custom or no composer-repository.
export-osv.php
Outdated
array_key_exists('link', $advisory) ? [ | ||
'type' => 'ADVISORY', | ||
'url' => $advisory['link'], | ||
] : null, |
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.
putting null
in the array looks wrong to me
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.
True. Since the validator enforces links anyway, I just skipped that check.
1b49ea4
to
5d2ade5
Compare
5d2ade5
to
1827365
Compare
Any new update here to get the fixed version into the list as well? |
@icanhazstring The PHP and OSV vulnerability schemes don't have a (Or maybe I'm misunderstanding your question?) |
@jaylinski was referring to the comment from @naderman about the Packagist advisory api about the fixed version. Or is it somewhat possible to get security issue listed with affected version and the next which fixes it? |
Just wanted to leave that here (probably you knew it already):
|
@ohader The packagist.org API already synchronizes and merges github's db and friendsofphp, e.g. see here: https://packagist.org/packages/guzzlehttp/guzzle/advisories?version=6278149 It would really just need someone to build an OSV compatible output for the data we collect there to have an OSV database for PHP. |
Fixes #576
This commit adds an automatic OSV export to the
osv
branch while keeping the current repository as is.Inspired by rustsec: https://github.com/rustsec/advisory-db/blob/main/.github/workflows/export-osv.yml
Preview
https://github.com/jaylinski/security-advisories/tree/osv
Possible improvements
Before merging
osv
branch with a readme similar to this one: https://github.com/rustsec/advisory-db/blob/osv/README.md