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

osv: account for event objects that have multiple streams #1428

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

crozzy
Copy link
Contributor

@crozzy crozzy commented Oct 18, 2024

It was discovered that some OSV documents can order minor releases in the same affected.ranges object. This meant that only ever counted the last range in a vulnerability. This change gathers range information for the affected product and creates a vulnerability per range.

@crozzy crozzy force-pushed the osv-account-for-multi-events branch from f1e69c0 to d9b0770 Compare October 18, 2024 21:57
It was discovered that some OSV documents can order minor releases in
the same affected.ranges object. This meant that only ever counted the
last range in a vulnerability. This change gathers range information for
the affected product and creates a vulnerability per range.

Signed-off-by: crozzy <[email protected]>
@crozzy crozzy force-pushed the osv-account-for-multi-events branch from d9b0770 to 6fea4d4 Compare October 18, 2024 21:58
@crozzy
Copy link
Contributor Author

crozzy commented Oct 18, 2024

Example ranges from an OSV document.

      "ranges": [
        {
          "type": "SEMVER",
          "events": [
            {
              "introduced": "0"
            },
            {
              "fixed": "1.21.12"
            },
            {
              "introduced": "1.22.0-0"
            },
            {
              "fixed": "1.22.5"
            }
          ]
        }
      ],

@crozzy crozzy marked this pull request as ready for review October 18, 2024 22:04
@crozzy crozzy requested a review from a team as a code owner October 18, 2024 22:04
@crozzy crozzy requested review from RTann and removed request for a team October 18, 2024 22:04
case ev.Introduced == "0": // -Inf
v.Range.Lower.Kind = `semver`
case ev.Introduced != "" && seenIntroduced:
vs = &rangeVer{rng: &claircore.Range{}}
Copy link
Contributor

Choose a reason for hiding this comment

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

if we are assuming it's well-formed (introduced, fixed, introduced, fixed, etc), then I think we can just do vs = &rangeVer{rng: &claircore.Range{}} unconditionally within case ev.Introduced and don't need the if ie == len(r.Events)-1 below. What do you think?

@@ -554,24 +558,40 @@ func (e *ecs) Insert(ctx context.Context, skipped *stats, name string, a *adviso
}
// This does some heavy assumptions about valid inputs.
ranges := make(url.Values)
Copy link
Contributor

Choose a reason for hiding this comment

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

does this need the same kind of treatment?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants