Skip to content

Commit e7094c2

Browse files
committed
osv: account for event objects that have multiple streams
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]>
1 parent 81480f4 commit e7094c2

File tree

2 files changed

+590
-64
lines changed

2 files changed

+590
-64
lines changed

updater/osv/osv.go

+97-57
Original file line numberDiff line numberDiff line change
@@ -492,6 +492,12 @@ func newECS(u string) ecs {
492492
}
493493
}
494494

495+
type rangeVer struct {
496+
fixedInVersion string
497+
semverRange *claircore.Range
498+
ecosystemRange url.Values
499+
}
500+
495501
func (e *ecs) Insert(ctx context.Context, skipped *stats, name string, a *advisory) (err error) {
496502
if a.GitOnly() {
497503
return nil
@@ -544,12 +550,10 @@ func (e *ecs) Insert(ctx context.Context, skipped *stats, name string, a *adviso
544550
proto.Links = b.String()
545551
for i := range a.Affected {
546552
af := &a.Affected[i]
547-
v := e.NewVulnerability()
548-
(*v) = proto
549553
for _, r := range af.Ranges {
554+
vers := []*rangeVer{}
550555
switch r.Type {
551556
case `SEMVER`:
552-
v.Range = &claircore.Range{}
553557
case `ECOSYSTEM`:
554558
b.Reset()
555559
case `GIT`:
@@ -561,25 +565,40 @@ func (e *ecs) Insert(ctx context.Context, skipped *stats, name string, a *adviso
561565
Msg("odd range type")
562566
}
563567
// This does some heavy assumptions about valid inputs.
564-
ranges := make(url.Values)
565-
for _, ev := range r.Events {
568+
vs := &rangeVer{semverRange: &claircore.Range{}, ecosystemRange: url.Values{}}
569+
var seenIntroduced bool
570+
for ie := range r.Events {
571+
ev := r.Events[ie]
566572
var err error
567573
switch r.Type {
568574
case `SEMVER`:
569575
var ver *semver.Version
570576
switch {
571-
case ev.Introduced == "0": // -Inf
572-
v.Range.Lower.Kind = `semver`
577+
case ev.Introduced != "" && seenIntroduced:
578+
vs = &rangeVer{semverRange: &claircore.Range{}}
579+
fallthrough
573580
case ev.Introduced != "":
574-
ver, err = semver.NewVersion(ev.Introduced)
575-
if err == nil {
576-
v.Range.Lower = claircore.FromSemver(ver)
581+
seenIntroduced = true
582+
switch {
583+
case ev.Introduced == "0": // -Inf
584+
vs.semverRange.Lower.Kind = `semver`
585+
default:
586+
ver, err = semver.NewVersion(ev.Introduced)
587+
if err == nil {
588+
vs.semverRange.Lower = claircore.FromSemver(ver)
589+
}
590+
}
591+
// Is this the last event? If so append the range and
592+
// implicitly terminate.
593+
if ie == len(r.Events)-1 {
594+
vers = append(vers, vs)
577595
}
596+
continue
578597
case ev.Fixed != "": // less than
579598
ver, err = semver.NewVersion(ev.Fixed)
580599
if err == nil {
581-
v.Range.Upper = claircore.FromSemver(ver)
582-
v.FixedInVersion = ver.Original()
600+
vs.semverRange.Upper = claircore.FromSemver(ver)
601+
vs.fixedInVersion = ver.Original()
583602
}
584603
case ev.LastAffected != "" && len(af.Versions) != 0: // less than equal to
585604
// TODO(hank) Should be able to convert this to a "less than."
@@ -594,11 +613,11 @@ func (e *ecs) Insert(ctx context.Context, skipped *stats, name string, a *adviso
594613
ver, err = semver.NewVersion(ev.LastAffected)
595614
if err == nil {
596615
nv := ver.IncPatch()
597-
v.Range.Upper = claircore.FromSemver(&nv)
616+
vs.semverRange.Upper = claircore.FromSemver(&nv)
598617
}
599618
case ev.Limit == "*": // +Inf
600-
v.Range.Upper.Kind = `semver`
601-
v.Range.Upper.V[0] = 65535
619+
vs.semverRange.Upper.Kind = `semver`
620+
vs.semverRange.Upper.V[0] = 65535
602621
case ev.Limit != "": // Something arbitrary
603622
zlog.Info(ctx).
604623
Str("which", "limit").
@@ -608,14 +627,26 @@ func (e *ecs) Insert(ctx context.Context, skipped *stats, name string, a *adviso
608627
case `ECOSYSTEM`:
609628
switch af.Package.Ecosystem {
610629
case ecosystemMaven, ecosystemPyPI, ecosystemRubyGems:
630+
vs.semverRange = nil
611631
switch {
612-
case ev.Introduced == "0":
632+
case ev.Introduced != "" && seenIntroduced:
633+
vs = &rangeVer{ecosystemRange: url.Values{}}
634+
fallthrough
613635
case ev.Introduced != "":
614-
ranges.Add("introduced", ev.Introduced)
636+
seenIntroduced = true
637+
switch {
638+
case ev.Introduced == "0":
639+
default:
640+
vs.ecosystemRange.Add("introduced", ev.Introduced)
641+
}
642+
if ie == len(r.Events)-1 {
643+
vers = append(vers, vs)
644+
}
645+
continue
615646
case ev.Fixed != "":
616-
ranges.Add("fixed", ev.Fixed)
647+
vs.ecosystemRange.Add("fixed", ev.Fixed)
617648
case ev.LastAffected != "":
618-
ranges.Add("lastAffected", ev.LastAffected)
649+
vs.ecosystemRange.Add("lastAffected", ev.LastAffected)
619650
}
620651
case ecosystemGo, ecosystemNPM:
621652
return fmt.Errorf(`unexpected "ECOSYSTEM" entry for %q ecosystem: %s`, af.Package.Ecosystem, a.ID)
@@ -624,7 +655,7 @@ func (e *ecs) Insert(ctx context.Context, skipped *stats, name string, a *adviso
624655
case ev.Introduced == "0": // -Inf
625656
case ev.Introduced != "":
626657
case ev.Fixed != "":
627-
v.FixedInVersion = ev.Fixed
658+
vs.fixedInVersion = ev.Fixed
628659
case ev.LastAffected != "":
629660
case ev.Limit == "*": // +Inf
630661
case ev.Limit != "":
@@ -634,49 +665,58 @@ func (e *ecs) Insert(ctx context.Context, skipped *stats, name string, a *adviso
634665
if err != nil {
635666
zlog.Warn(ctx).Err(err).Msg("event version error")
636667
}
668+
vers = append(vers, vs)
637669
}
638-
if len(ranges) > 0 {
639-
switch af.Package.Ecosystem {
640-
case ecosystemMaven, ecosystemPyPI, ecosystemRubyGems:
641-
v.FixedInVersion = ranges.Encode()
670+
// Now we need to cycle through the ranges and create the vulnerabilities.
671+
for _, ve := range vers {
672+
v := e.NewVulnerability()
673+
(*v) = proto
674+
v.Range = ve.semverRange
675+
v.FixedInVersion = ve.fixedInVersion
676+
if len(ve.ecosystemRange) > 0 {
677+
switch af.Package.Ecosystem {
678+
case ecosystemMaven, ecosystemPyPI, ecosystemRubyGems:
679+
v.FixedInVersion = ve.ecosystemRange.Encode()
680+
}
642681
}
643-
}
644682

645-
if r := v.Range; r != nil {
646-
// We have an implicit +Inf range if there's a single event,
647-
// this should catch it?
648-
if r.Upper.Kind == "" {
649-
r.Upper.Kind = r.Lower.Kind
650-
r.Upper.V[0] = 65535
683+
if r := v.Range; r != nil {
684+
// We have an implicit +Inf range if there's a single event,
685+
// this should catch it?
686+
if r.Upper.Kind == "" {
687+
r.Upper.Kind = r.Lower.Kind
688+
r.Upper.V[0] = 65535
689+
}
690+
if r.Lower.Compare(&r.Upper) == 1 {
691+
e.RemoveVulnerability(v)
692+
skipped.Ignored = append(skipped.Ignored, fmt.Sprintf("%s(%s,%s)", a.ID, r.Lower.String(), r.Upper.String()))
693+
continue
694+
}
651695
}
652-
if r.Lower.Compare(&r.Upper) == 1 {
653-
e.RemoveVulnerability(v)
654-
skipped.Ignored = append(skipped.Ignored, fmt.Sprintf("%s(%s,%s)", a.ID, r.Lower.String(), r.Upper.String()))
655-
continue
696+
var vs string
697+
switch r.Type {
698+
case `ECOSYSTEM`:
699+
vs = b.String()
700+
}
701+
pkgName := af.Package.PURL
702+
switch af.Package.Ecosystem {
703+
case ecosystemGo, ecosystemMaven, ecosystemNPM, ecosystemPyPI, ecosystemRubyGems:
704+
pkgName = af.Package.Name
705+
}
706+
pkg, novel := e.LookupPackage(pkgName, vs)
707+
v.Package = pkg
708+
switch af.Package.Ecosystem {
709+
case ecosystemGo, ecosystemMaven, ecosystemNPM, ecosystemPyPI, ecosystemRubyGems:
710+
v.Package.Kind = claircore.BINARY
711+
}
712+
if novel {
713+
pkg.RepositoryHint = af.Package.Ecosystem
714+
}
715+
if repo := e.LookupRepository(name); repo != nil {
716+
v.Repo = repo
656717
}
657718
}
658-
var vs string
659-
switch r.Type {
660-
case `ECOSYSTEM`:
661-
vs = b.String()
662-
}
663-
pkgName := af.Package.PURL
664-
switch af.Package.Ecosystem {
665-
case ecosystemGo, ecosystemMaven, ecosystemNPM, ecosystemPyPI, ecosystemRubyGems:
666-
pkgName = af.Package.Name
667-
}
668-
pkg, novel := e.LookupPackage(pkgName, vs)
669-
v.Package = pkg
670-
switch af.Package.Ecosystem {
671-
case ecosystemGo, ecosystemMaven, ecosystemNPM, ecosystemPyPI, ecosystemRubyGems:
672-
v.Package.Kind = claircore.BINARY
673-
}
674-
if novel {
675-
pkg.RepositoryHint = af.Package.Ecosystem
676-
}
677-
if repo := e.LookupRepository(name); repo != nil {
678-
v.Repo = repo
679-
}
719+
680720
}
681721
}
682722
return nil

0 commit comments

Comments
 (0)