diff --git a/updater/osv/osv.go b/updater/osv/osv.go index 354e8e5e1..596608902 100644 --- a/updater/osv/osv.go +++ b/updater/osv/osv.go @@ -492,6 +492,12 @@ func newECS(u string) ecs { } } +type rangeVer struct { + fixedInVersion string + semverRange *claircore.Range + ecosystemRange url.Values +} + func (e *ecs) Insert(ctx context.Context, skipped *stats, name string, a *advisory) (err error) { if a.GitOnly() { return nil @@ -544,12 +550,10 @@ func (e *ecs) Insert(ctx context.Context, skipped *stats, name string, a *adviso proto.Links = b.String() for i := range a.Affected { af := &a.Affected[i] - v := e.NewVulnerability() - (*v) = proto for _, r := range af.Ranges { + vers := []*rangeVer{} switch r.Type { case `SEMVER`: - v.Range = &claircore.Range{} case `ECOSYSTEM`: b.Reset() case `GIT`: @@ -561,25 +565,40 @@ func (e *ecs) Insert(ctx context.Context, skipped *stats, name string, a *adviso Msg("odd range type") } // This does some heavy assumptions about valid inputs. - ranges := make(url.Values) - for _, ev := range r.Events { + vs := &rangeVer{semverRange: &claircore.Range{}, ecosystemRange: url.Values{}} + var seenIntroduced bool + for ie := range r.Events { + ev := r.Events[ie] var err error switch r.Type { case `SEMVER`: var ver *semver.Version switch { - case ev.Introduced == "0": // -Inf - v.Range.Lower.Kind = `semver` + case ev.Introduced != "" && seenIntroduced: + vs = &rangeVer{semverRange: &claircore.Range{}} + fallthrough case ev.Introduced != "": - ver, err = semver.NewVersion(ev.Introduced) - if err == nil { - v.Range.Lower = claircore.FromSemver(ver) + seenIntroduced = true + switch { + case ev.Introduced == "0": // -Inf + vs.semverRange.Lower.Kind = `semver` + default: + ver, err = semver.NewVersion(ev.Introduced) + if err == nil { + vs.semverRange.Lower = claircore.FromSemver(ver) + } + } + // Is this the last event? If so append the range and + // implicitly terminate. + if ie == len(r.Events)-1 { + vers = append(vers, vs) } + continue case ev.Fixed != "": // less than ver, err = semver.NewVersion(ev.Fixed) if err == nil { - v.Range.Upper = claircore.FromSemver(ver) - v.FixedInVersion = ver.Original() + vs.semverRange.Upper = claircore.FromSemver(ver) + vs.fixedInVersion = ver.Original() } case ev.LastAffected != "" && len(af.Versions) != 0: // less than equal to // 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 ver, err = semver.NewVersion(ev.LastAffected) if err == nil { nv := ver.IncPatch() - v.Range.Upper = claircore.FromSemver(&nv) + vs.semverRange.Upper = claircore.FromSemver(&nv) } case ev.Limit == "*": // +Inf - v.Range.Upper.Kind = `semver` - v.Range.Upper.V[0] = 65535 + vs.semverRange.Upper.Kind = `semver` + vs.semverRange.Upper.V[0] = 65535 case ev.Limit != "": // Something arbitrary zlog.Info(ctx). Str("which", "limit"). @@ -608,14 +627,26 @@ func (e *ecs) Insert(ctx context.Context, skipped *stats, name string, a *adviso case `ECOSYSTEM`: switch af.Package.Ecosystem { case ecosystemMaven, ecosystemPyPI, ecosystemRubyGems: + vs.semverRange = nil switch { - case ev.Introduced == "0": + case ev.Introduced != "" && seenIntroduced: + vs = &rangeVer{ecosystemRange: url.Values{}} + fallthrough case ev.Introduced != "": - ranges.Add("introduced", ev.Introduced) + seenIntroduced = true + switch { + case ev.Introduced == "0": + default: + vs.ecosystemRange.Add("introduced", ev.Introduced) + } + if ie == len(r.Events)-1 { + vers = append(vers, vs) + } + continue case ev.Fixed != "": - ranges.Add("fixed", ev.Fixed) + vs.ecosystemRange.Add("fixed", ev.Fixed) case ev.LastAffected != "": - ranges.Add("lastAffected", ev.LastAffected) + vs.ecosystemRange.Add("lastAffected", ev.LastAffected) } case ecosystemGo, ecosystemNPM: 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 case ev.Introduced == "0": // -Inf case ev.Introduced != "": case ev.Fixed != "": - v.FixedInVersion = ev.Fixed + vs.fixedInVersion = ev.Fixed case ev.LastAffected != "": case ev.Limit == "*": // +Inf case ev.Limit != "": @@ -634,49 +665,58 @@ func (e *ecs) Insert(ctx context.Context, skipped *stats, name string, a *adviso if err != nil { zlog.Warn(ctx).Err(err).Msg("event version error") } + vers = append(vers, vs) } - if len(ranges) > 0 { - switch af.Package.Ecosystem { - case ecosystemMaven, ecosystemPyPI, ecosystemRubyGems: - v.FixedInVersion = ranges.Encode() + // Now we need to cycle through the ranges and create the vulnerabilities. + for _, ve := range vers { + v := e.NewVulnerability() + (*v) = proto + v.Range = ve.semverRange + v.FixedInVersion = ve.fixedInVersion + if len(ve.ecosystemRange) > 0 { + switch af.Package.Ecosystem { + case ecosystemMaven, ecosystemPyPI, ecosystemRubyGems: + v.FixedInVersion = ve.ecosystemRange.Encode() + } } - } - if r := v.Range; r != nil { - // We have an implicit +Inf range if there's a single event, - // this should catch it? - if r.Upper.Kind == "" { - r.Upper.Kind = r.Lower.Kind - r.Upper.V[0] = 65535 + if r := v.Range; r != nil { + // We have an implicit +Inf range if there's a single event, + // this should catch it? + if r.Upper.Kind == "" { + r.Upper.Kind = r.Lower.Kind + r.Upper.V[0] = 65535 + } + if r.Lower.Compare(&r.Upper) == 1 { + e.RemoveVulnerability(v) + skipped.Ignored = append(skipped.Ignored, fmt.Sprintf("%s(%s,%s)", a.ID, r.Lower.String(), r.Upper.String())) + continue + } } - if r.Lower.Compare(&r.Upper) == 1 { - e.RemoveVulnerability(v) - skipped.Ignored = append(skipped.Ignored, fmt.Sprintf("%s(%s,%s)", a.ID, r.Lower.String(), r.Upper.String())) - continue + var vs string + switch r.Type { + case `ECOSYSTEM`: + vs = b.String() + } + pkgName := af.Package.PURL + switch af.Package.Ecosystem { + case ecosystemGo, ecosystemMaven, ecosystemNPM, ecosystemPyPI, ecosystemRubyGems: + pkgName = af.Package.Name + } + pkg, novel := e.LookupPackage(pkgName, vs) + v.Package = pkg + switch af.Package.Ecosystem { + case ecosystemGo, ecosystemMaven, ecosystemNPM, ecosystemPyPI, ecosystemRubyGems: + v.Package.Kind = claircore.BINARY + } + if novel { + pkg.RepositoryHint = af.Package.Ecosystem + } + if repo := e.LookupRepository(name); repo != nil { + v.Repo = repo } } - var vs string - switch r.Type { - case `ECOSYSTEM`: - vs = b.String() - } - pkgName := af.Package.PURL - switch af.Package.Ecosystem { - case ecosystemGo, ecosystemMaven, ecosystemNPM, ecosystemPyPI, ecosystemRubyGems: - pkgName = af.Package.Name - } - pkg, novel := e.LookupPackage(pkgName, vs) - v.Package = pkg - switch af.Package.Ecosystem { - case ecosystemGo, ecosystemMaven, ecosystemNPM, ecosystemPyPI, ecosystemRubyGems: - v.Package.Kind = claircore.BINARY - } - if novel { - pkg.RepositoryHint = af.Package.Ecosystem - } - if repo := e.LookupRepository(name); repo != nil { - v.Repo = repo - } + } } return nil diff --git a/updater/osv/osv_test.go b/updater/osv/osv_test.go index 3a4186f55..09bda1db4 100644 --- a/updater/osv/osv_test.go +++ b/updater/osv/osv_test.go @@ -18,6 +18,9 @@ import ( "strings" "testing" + "github.com/Masterminds/semver" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/quay/zlog" "github.com/quay/claircore" @@ -138,18 +141,509 @@ func TestParse(t *testing.T) { } t.Logf("parsed %d vulnerabilities", len(vs)) if len(vs) != 0 { - t.Log("first one:") - var buf bytes.Buffer - enc := json.NewEncoder(&buf) - enc.SetIndent("", "\t") - if err := enc.Encode(vs[0]); err != nil { - t.Error(err) + for _, v := range vs { + var buf bytes.Buffer + enc := json.NewEncoder(&buf) + enc.SetIndent("", "\t") + if err := enc.Encode(v); err != nil { + t.Error(err) + } + t.Log(buf.String()) } - t.Log(buf.String()) } } } +var insertTestCases = []struct { + name string + ad *advisory + expectedVulns []claircore.Vulnerability +}{ + { + name: "normal", + ad: &advisory{ + ID: "test1", + Affected: []affected{ + { + Package: _package{ + Ecosystem: "go", + Name: "something", + }, + Ranges: []_range{ + { + Type: "SEMVER", + Events: []rangeEvent{ + { + Introduced: "0", + }, + { + Fixed: "0.4.0", + }, + }, + }, + }, + }, + }, + }, + expectedVulns: []claircore.Vulnerability{ + { + Name: "test1", + Updater: "test", + Range: &claircore.Range{ + Lower: claircore.FromSemver(semver.MustParse("0.0.0")), + Upper: claircore.FromSemver(semver.MustParse("0.4.0")), + }, + FixedInVersion: "0.4.0", + }, + }, + }, + { + name: "unfixed", + ad: &advisory{ + ID: "test1", + Affected: []affected{ + { + Package: _package{ + Ecosystem: "go", + Name: "something", + }, + Ranges: []_range{ + { + Type: "SEMVER", + Events: []rangeEvent{ + { + Introduced: "0", + }, + }, + }, + }, + }, + }, + }, + expectedVulns: []claircore.Vulnerability{ + { + Name: "test1", + Updater: "test", + Range: &claircore.Range{ + Lower: claircore.FromSemver(semver.MustParse("0.0.0")), + Upper: claircore.Version{ + Kind: "semver", + V: [10]int32{65535, 0, 0, 0, 0, 0, 0, 0, 0, 0}, + }, + }, + FixedInVersion: "", + }, + }, + }, + { + name: "two_affected", + ad: &advisory{ + ID: "test1", + Affected: []affected{ + { + Package: _package{ + Ecosystem: "go", + Name: "something", + }, + Ranges: []_range{ + { + Type: "SEMVER", + Events: []rangeEvent{ + { + Introduced: "0", + }, + { + Fixed: "0.4.10", + }, + }, + }, + }, + }, + { + Package: _package{ + Ecosystem: "go", + Name: "something", + }, + Ranges: []_range{ + { + Type: "SEMVER", + Events: []rangeEvent{ + { + Introduced: "0.5.0", + }, + { + Fixed: "0.5.3", + }, + }, + }, + }, + }, + }, + }, + expectedVulns: []claircore.Vulnerability{ + { + Name: "test1", + Updater: "test", + Range: &claircore.Range{ + Lower: claircore.FromSemver(semver.MustParse("0.0.0")), + Upper: claircore.FromSemver(semver.MustParse("0.4.10")), + }, + FixedInVersion: "0.4.10", + }, + { + Name: "test1", + Updater: "test", + Range: &claircore.Range{ + Lower: claircore.FromSemver(semver.MustParse("0.5.0")), + Upper: claircore.FromSemver(semver.MustParse("0.5.3")), + }, + FixedInVersion: "0.5.3", + }, + }, + }, + { + name: "three_fixes", + ad: &advisory{ + ID: "test1", + Affected: []affected{ + { + Package: _package{ + Ecosystem: "go", + Name: "something", + }, + Ranges: []_range{ + { + Type: "SEMVER", + Events: []rangeEvent{ + { + Introduced: "0", + }, + { + Fixed: "2.1.16", + }, + { + Introduced: "2.2.0", + }, + { + Fixed: "2.2.10", + }, + { + Introduced: "2.3.0", + }, + { + Fixed: "2.3.5", + }, + }, + }, + }, + }, + }, + }, + expectedVulns: []claircore.Vulnerability{ + { + Name: "test1", + Updater: "test", + Range: &claircore.Range{ + Lower: claircore.FromSemver(semver.MustParse("0.0.0")), + Upper: claircore.FromSemver(semver.MustParse("2.1.16")), + }, + FixedInVersion: "2.1.16", + }, + { + Name: "test1", + Updater: "test", + Range: &claircore.Range{ + Lower: claircore.FromSemver(semver.MustParse("2.2.0")), + Upper: claircore.FromSemver(semver.MustParse("2.2.10")), + }, + FixedInVersion: "2.2.10", + }, + { + Name: "test1", + Updater: "test", + Range: &claircore.Range{ + Lower: claircore.FromSemver(semver.MustParse("2.3.0")), + Upper: claircore.FromSemver(semver.MustParse("2.3.5")), + }, + FixedInVersion: "2.3.5", + }, + }, + }, + { + name: "two_fixes_one_unfixed", + ad: &advisory{ + ID: "test1", + Affected: []affected{ + { + Package: _package{ + Ecosystem: "go", + Name: "something", + }, + Ranges: []_range{ + { + Type: "SEMVER", + Events: []rangeEvent{ + { + Introduced: "0", + }, + { + Fixed: "2.1.16", + }, + { + Introduced: "2.2.0", + }, + { + Fixed: "2.2.10", + }, + { + Introduced: "2.3.0", + }, + }, + }, + }, + }, + }, + }, + expectedVulns: []claircore.Vulnerability{ + { + Name: "test1", + Updater: "test", + Range: &claircore.Range{ + Lower: claircore.FromSemver(semver.MustParse("0.0.0")), + Upper: claircore.FromSemver(semver.MustParse("2.1.16")), + }, + FixedInVersion: "2.1.16", + }, + { + Name: "test1", + Updater: "test", + Range: &claircore.Range{ + Lower: claircore.FromSemver(semver.MustParse("2.2.0")), + Upper: claircore.FromSemver(semver.MustParse("2.2.10")), + }, + FixedInVersion: "2.2.10", + }, + { + Name: "test1", + Updater: "test", + Range: &claircore.Range{ + Lower: claircore.FromSemver(semver.MustParse("2.3.0")), + Upper: claircore.Version{ + Kind: "semver", + V: [10]int32{65535, 0, 0, 0, 0, 0, 0, 0, 0, 0}, + }, + }, + FixedInVersion: "", + }, + }, + }, + { + // In this situation we're just expecting the last one. + name: "two_consecutive_introduced_invalid", + ad: &advisory{ + ID: "test1", + Affected: []affected{ + { + Package: _package{ + Ecosystem: "go", + Name: "something", + }, + Ranges: []_range{ + { + Type: "SEMVER", + Events: []rangeEvent{ + { + Introduced: "2.2.0", + }, + { + Introduced: "2.3.0", + }, + }, + }, + }, + }, + }, + }, + expectedVulns: []claircore.Vulnerability{ + { + Name: "test1", + Updater: "test", + Range: &claircore.Range{ + Lower: claircore.FromSemver(semver.MustParse("2.3.0")), + Upper: claircore.Version{ + Kind: "semver", + V: [10]int32{65535, 0, 0, 0, 0, 0, 0, 0, 0, 0}, + }, + }, + FixedInVersion: "", + }, + }, + }, + { + name: "ecosystem_multi", + ad: &advisory{ + ID: "test1", + Affected: []affected{ + { + Package: _package{ + Ecosystem: "PyPI", + Name: "cherrypy", + }, + Ranges: []_range{ + { + Type: "ECOSYSTEM", + Events: []rangeEvent{ + { + Introduced: "0", + }, + { + Fixed: "2.1.1", + }, + { + Introduced: "3.0", + }, + { + Fixed: "3.0.2", + }, + }, + }, + }, + }, + }, + }, + expectedVulns: []claircore.Vulnerability{ + { + Name: "test1", + Updater: "test", + Range: nil, + FixedInVersion: "fixed=2.1.1", + }, + { + Name: "test1", + Updater: "test", + Range: nil, + FixedInVersion: "fixed=3.0.2&introduced=3.0", + }, + }, + }, + { + name: "ecosystem_unfixed", + ad: &advisory{ + ID: "test1", + Affected: []affected{ + { + Package: _package{ + Ecosystem: "PyPI", + Name: "cherrypy", + }, + Ranges: []_range{ + { + Type: "ECOSYSTEM", + Events: []rangeEvent{ + { + Introduced: "3.0", + }, + }, + }, + }, + }, + }, + }, + expectedVulns: []claircore.Vulnerability{ + { + Name: "test1", + Updater: "test", + Range: nil, + FixedInVersion: "introduced=3.0", + }, + }, + }, + { + name: "same package different ranges", + ad: &advisory{ + ID: "test1", + Affected: []affected{ + { + Package: _package{ + Ecosystem: "go", + Name: "something", + }, + Ranges: []_range{ + { + Type: "SEMVER", + Events: []rangeEvent{ + { + Introduced: "0", + }, + { + Fixed: "0.4.0", + }, + }, + }, + { + Type: "SEMVER", + Events: []rangeEvent{ + { + Introduced: "1.0.0", + }, + { + Fixed: "1.2.0", + }, + }, + }, + }, + }, + }, + }, + expectedVulns: []claircore.Vulnerability{ + { + Name: "test1", + Updater: "test", + Range: &claircore.Range{ + Lower: claircore.FromSemver(semver.MustParse("0.0.0")), + Upper: claircore.FromSemver(semver.MustParse("0.4.0")), + }, + FixedInVersion: "0.4.0", + }, + { + Name: "test1", + Updater: "test", + Range: &claircore.Range{ + Lower: claircore.FromSemver(semver.MustParse("1.0.0")), + Upper: claircore.FromSemver(semver.MustParse("1.2.0")), + }, + FixedInVersion: "1.2.0", + }, + }, + }, +} + +// cmpIgnore will ignore everything expect the Name, Updater, Range and FixedInVersion. +var cmpIgnore = cmpopts.IgnoreFields( + claircore.Vulnerability{}, "ID", "Updater", "Description", "Severity", "NormalizedSeverity", "Package", "Repo") + +func TestInsert(t *testing.T) { + ctx := zlog.Test(context.Background(), t) + + for _, tt := range insertTestCases { + t.Run(tt.name, func(t *testing.T) { + ecs := newECS("test") + + err := ecs.Insert(ctx, nil, "", tt.ad) + if err != nil { + t.Error("got error Inserting advisory", err) + } + if len(ecs.Vulnerability) != len(tt.expectedVulns) { + t.Fatalf("should have %d vulnerability but got %d", len(tt.expectedVulns), len(ecs.Vulnerability)) + } + + if !cmp.Equal(ecs.Vulnerability, tt.expectedVulns, cmpIgnore) { + t.Error(cmp.Diff(ecs.Vulnerability, tt.expectedVulns, cmpIgnore)) + } + }) + } +} + var severityTestCases = []struct { name string a *advisory