From fc2b50a6f23b8776216625425e5028886de4f1a2 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Sun, 30 Jul 2023 10:18:47 +1200 Subject: [PATCH] fix: ensure that affected entries are sorted before comparing --- pkg/database/osv.go | 24 +++++ pkg/database/osv_test.go | 190 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 214 insertions(+) diff --git a/pkg/database/osv.go b/pkg/database/osv.go index df38116c..5c0f60bb 100644 --- a/pkg/database/osv.go +++ b/pkg/database/osv.go @@ -8,6 +8,7 @@ import ( "github.com/g-rath/osv-detector/pkg/semantic" "os" "regexp" + "sort" "strings" "time" "unicode" @@ -58,6 +59,22 @@ type AffectsRange struct { Events []RangeEvent `json:"events"` } +func (e RangeEvent) version() string { + if e.Introduced != "" { + return e.Introduced + } + + if e.Fixed != "" { + return e.Fixed + } + + if e.LastAffected != "" { + return e.LastAffected + } + + return "" +} + func (ar AffectsRange) containsVersion(pkg internal.PackageDetails) bool { if ar.Type != TypeEcosystem && ar.Type != TypeSemver { return false @@ -69,6 +86,13 @@ func (ar AffectsRange) containsVersion(pkg internal.PackageDetails) bool { vp := semantic.MustParse(pkg.Version, pkg.CompareAs) + sort.Slice(ar.Events, func(i, j int) bool { + a := ar.Events[i] + b := ar.Events[j] + + return semantic.MustParse(a.version(), pkg.CompareAs).CompareStr(b.version()) < 0 + }) + var affected bool for _, e := range ar.Events { if affected { diff --git a/pkg/database/osv_test.go b/pkg/database/osv_test.go index 7134aa28..c8949317 100644 --- a/pkg/database/osv_test.go +++ b/pkg/database/osv_test.go @@ -280,6 +280,40 @@ func TestOSV_IsAffected_AffectsWithEcosystem_SingleAffected(t *testing.T) { // an empty version should always be treated as affected expectIsAffected(t, osv, "", true) + // multiple fixes and introduced, shuffled + osv = buildOSVWithAffected( + database.Affected{ + Package: database.Package{Ecosystem: lockfile.NpmEcosystem, Name: "my-package"}, + Ranges: []database.AffectsRange{ + buildEcosystemAffectsRange( + database.RangeEvent{Introduced: "0"}, + database.RangeEvent{Introduced: "2.1.0"}, + database.RangeEvent{Fixed: "3.2.0"}, + database.RangeEvent{Fixed: "1"}, + ), + }, + }, + ) + + for _, v := range []string{"0.0.0", "0.1.0", "0.0.0.1", "1.0.0-rc"} { + expectIsAffected(t, osv, v, true) + } + + for _, v := range []string{"1.0.0", "1.1.0", "2.0.0rc2", "2.0.1"} { + expectIsAffected(t, osv, v, false) + } + + for _, v := range []string{"2.1.1", "2.3.4", "3.0.0", "3.0.0-rc"} { + expectIsAffected(t, osv, v, true) + } + + for _, v := range []string{"3.2.0", "3.2.1", "4.0.0"} { + expectIsAffected(t, osv, v, false) + } + + // an empty version should always be treated as affected + expectIsAffected(t, osv, "", true) + // "LastAffected: 1" means all versions after this are not vulnerable osv = buildOSVWithAffected( database.Affected{ @@ -337,6 +371,40 @@ func TestOSV_IsAffected_AffectsWithEcosystem_SingleAffected(t *testing.T) { // an empty version should always be treated as affected expectIsAffected(t, osv, "", true) + + // mix of fixes, last_known_affected, and introduced, shuffled + osv = buildOSVWithAffected( + database.Affected{ + Package: database.Package{Ecosystem: lockfile.NpmEcosystem, Name: "my-package"}, + Ranges: []database.AffectsRange{ + buildEcosystemAffectsRange( + database.RangeEvent{Introduced: "0"}, + database.RangeEvent{Introduced: "2.1.0"}, + database.RangeEvent{Fixed: "1"}, + database.RangeEvent{LastAffected: "3.1.9"}, + ), + }, + }, + ) + + for _, v := range []string{"0.0.0", "0.1.0", "0.0.0.1", "1.0.0-rc"} { + expectIsAffected(t, osv, v, true) + } + + for _, v := range []string{"1.0.0", "1.1.0", "2.0.0rc2", "2.0.1"} { + expectIsAffected(t, osv, v, false) + } + + for _, v := range []string{"2.1.1", "2.3.4", "3.0.0", "3.0.0-rc", "3.1.9"} { + expectIsAffected(t, osv, v, true) + } + + for _, v := range []string{"3.2.0", "3.2.1", "4.0.0"} { + expectIsAffected(t, osv, v, false) + } + + // an empty version should always be treated as affected + expectIsAffected(t, osv, "", true) } func TestOSV_IsAffected_AffectsWithEcosystem_MultipleAffected(t *testing.T) { @@ -394,6 +462,60 @@ func TestOSV_IsAffected_AffectsWithEcosystem_MultipleAffected(t *testing.T) { // an empty version should always be treated as affected expectIsAffected(t, osv, "", true) + + // shuffled + osv = buildOSVWithAffected( + database.Affected{ + Package: database.Package{Ecosystem: lockfile.NpmEcosystem, Name: "my-package"}, + Ranges: []database.AffectsRange{ + buildEcosystemAffectsRange( + database.RangeEvent{Fixed: "1"}, + database.RangeEvent{Introduced: "0"}, + ), + }, + }, + database.Affected{ + Package: database.Package{Ecosystem: lockfile.NpmEcosystem, Name: "my-package"}, + Ranges: []database.AffectsRange{ + buildEcosystemAffectsRange( + database.RangeEvent{Fixed: "3.2.0"}, + database.RangeEvent{Introduced: "2.1.0"}, + ), + }, + }, + database.Affected{ + Package: database.Package{Ecosystem: lockfile.NpmEcosystem, Name: "my-package"}, + Ranges: []database.AffectsRange{ + buildEcosystemAffectsRange( + database.RangeEvent{LastAffected: "3.5.0"}, + database.RangeEvent{Introduced: "3.3.0"}, + ), + }, + }, + ) + + for _, v := range []string{"0.0.0", "0.1.0", "0.0.0.1", "1.0.0-rc"} { + expectIsAffected(t, osv, v, true) + } + + for _, v := range []string{"1.0.0", "1.1.0", "2.0.0rc2", "2.0.1"} { + expectIsAffected(t, osv, v, false) + } + + for _, v := range []string{"2.1.1", "2.3.4", "3.0.0", "3.0.0-rc"} { + expectIsAffected(t, osv, v, true) + } + + for _, v := range []string{"3.2.0", "3.2.1", "4.0.0"} { + expectIsAffected(t, osv, v, false) + } + + for _, v := range []string{"3.3.1", "3.4.5"} { + expectIsAffected(t, osv, v, true) + } + + // an empty version should always be treated as affected + expectIsAffected(t, osv, "", true) } func TestOSV_IsAffected_AffectsWithEcosystem_PipNamesAreNormalised(t *testing.T) { @@ -540,6 +662,40 @@ func TestOSV_IsAffected_AffectsWithSemver_SingleAffected(t *testing.T) { // an empty version should always be treated as affected expectIsAffected(t, osv, "", true) + // multiple fixes and introduced, shuffled + osv = buildOSVWithAffected( + database.Affected{ + Package: database.Package{Ecosystem: lockfile.NpmEcosystem, Name: "my-package"}, + Ranges: []database.AffectsRange{ + buildSemverAffectsRange( + database.RangeEvent{Fixed: "1"}, + database.RangeEvent{Fixed: "3.2.0"}, + database.RangeEvent{Introduced: "0"}, + database.RangeEvent{Introduced: "2.1.0"}, + ), + }, + }, + ) + + for _, v := range []string{"0.0.0", "0.1.0", "0.0.0.1", "1.0.0-rc"} { + expectIsAffected(t, osv, v, true) + } + + for _, v := range []string{"1.0.0", "1.1.0", "2.0.0rc2", "2.0.1"} { + expectIsAffected(t, osv, v, false) + } + + for _, v := range []string{"2.1.1", "2.3.4", "3.0.0", "3.0.0-rc"} { + expectIsAffected(t, osv, v, true) + } + + for _, v := range []string{"3.2.0", "3.2.1", "4.0.0"} { + expectIsAffected(t, osv, v, false) + } + + // an empty version should always be treated as affected + expectIsAffected(t, osv, "", true) + // "LastAffected: 1" means all versions after this are not vulnerable osv = buildOSVWithAffected( database.Affected{ @@ -594,6 +750,40 @@ func TestOSV_IsAffected_AffectsWithSemver_SingleAffected(t *testing.T) { // an empty version should always be treated as affected expectIsAffected(t, osv, "", true) + + // mix of fixes, last_known_affected, and introduced, shuffled + osv = buildOSVWithAffected( + database.Affected{ + Package: database.Package{Ecosystem: lockfile.NpmEcosystem, Name: "my-package"}, + Ranges: []database.AffectsRange{ + buildSemverAffectsRange( + database.RangeEvent{Introduced: "2.1.0"}, + database.RangeEvent{Introduced: "0"}, + database.RangeEvent{LastAffected: "3.1.9"}, + database.RangeEvent{Fixed: "1"}, + ), + }, + }, + ) + + for _, v := range []string{"0.0.0", "0.1.0", "0.0.0.1", "1.0.0-rc"} { + expectIsAffected(t, osv, v, true) + } + + for _, v := range []string{"1.0.0", "1.1.0", "2.0.0rc2", "2.0.1"} { + expectIsAffected(t, osv, v, false) + } + + for _, v := range []string{"2.1.1", "2.3.4", "3.0.0", "3.0.0-rc"} { + expectIsAffected(t, osv, v, true) + } + + for _, v := range []string{"3.2.0", "3.2.1", "4.0.0"} { + expectIsAffected(t, osv, v, false) + } + + // an empty version should always be treated as affected + expectIsAffected(t, osv, "", true) } func TestOSV_IsAffected_AffectsWithSemver_MultipleAffected(t *testing.T) {