Skip to content

Commit e20e642

Browse files
G-RathG-Rath
authored
fix: ensure that affected entries are in order before comparing (#198)
* fix: ensure that affected entries are sorted before comparing * fix: sort "introduced 0" events before any other version
1 parent 8692cb0 commit e20e642

File tree

2 files changed

+261
-0
lines changed

2 files changed

+261
-0
lines changed

‎pkg/database/osv.go‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"github.com/g-rath/osv-detector/pkg/semantic"
99
"os"
1010
"regexp"
11+
"sort"
1112
"strings"
1213
"time"
1314
"unicode"
@@ -58,6 +59,22 @@ type AffectsRange struct {
5859
Events []RangeEvent `json:"events"`
5960
}
6061

62+
func (e RangeEvent) version() string {
63+
if e.Introduced != "" {
64+
return e.Introduced
65+
}
66+
67+
if e.Fixed != "" {
68+
return e.Fixed
69+
}
70+
71+
if e.LastAffected != "" {
72+
return e.LastAffected
73+
}
74+
75+
return ""
76+
}
77+
6178
func (ar AffectsRange) containsVersion(pkg internal.PackageDetails) bool {
6279
if ar.Type != TypeEcosystem && ar.Type != TypeSemver {
6380
return false
@@ -69,6 +86,21 @@ func (ar AffectsRange) containsVersion(pkg internal.PackageDetails) bool {
6986

7087
vp := semantic.MustParse(pkg.Version, pkg.CompareAs)
7188

89+
sort.Slice(ar.Events, func(i, j int) bool {
90+
a := ar.Events[i]
91+
b := ar.Events[j]
92+
93+
if a.Introduced == "0" {
94+
return true
95+
}
96+
97+
if b.Introduced == "0" {
98+
return false
99+
}
100+
101+
return semantic.MustParse(a.version(), pkg.CompareAs).CompareStr(b.version()) < 0
102+
})
103+
72104
var affected bool
73105
for _, e := range ar.Events {
74106
if affected {

‎pkg/database/osv_test.go‎

Lines changed: 229 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@ func expectIsAffected(t *testing.T, osv database.OSV, version string, expectAffe
2727
}
2828

2929
if osv.IsAffected(pkg) != expectAffected {
30+
if version == "" {
31+
version = "<empty>"
32+
}
33+
3034
if expectAffected {
3135
t.Errorf("Expected OSV to affect package version %s but it did not", version)
3236
} else {
@@ -280,6 +284,40 @@ func TestOSV_IsAffected_AffectsWithEcosystem_SingleAffected(t *testing.T) {
280284
// an empty version should always be treated as affected
281285
expectIsAffected(t, osv, "", true)
282286

287+
// multiple fixes and introduced, shuffled
288+
osv = buildOSVWithAffected(
289+
database.Affected{
290+
Package: database.Package{Ecosystem: lockfile.NpmEcosystem, Name: "my-package"},
291+
Ranges: []database.AffectsRange{
292+
buildEcosystemAffectsRange(
293+
database.RangeEvent{Introduced: "0"},
294+
database.RangeEvent{Introduced: "2.1.0"},
295+
database.RangeEvent{Fixed: "3.2.0"},
296+
database.RangeEvent{Fixed: "1"},
297+
),
298+
},
299+
},
300+
)
301+
302+
for _, v := range []string{"0.0.0", "0.1.0", "0.0.0.1", "1.0.0-rc"} {
303+
expectIsAffected(t, osv, v, true)
304+
}
305+
306+
for _, v := range []string{"1.0.0", "1.1.0", "2.0.0rc2", "2.0.1"} {
307+
expectIsAffected(t, osv, v, false)
308+
}
309+
310+
for _, v := range []string{"2.1.1", "2.3.4", "3.0.0", "3.0.0-rc"} {
311+
expectIsAffected(t, osv, v, true)
312+
}
313+
314+
for _, v := range []string{"3.2.0", "3.2.1", "4.0.0"} {
315+
expectIsAffected(t, osv, v, false)
316+
}
317+
318+
// an empty version should always be treated as affected
319+
expectIsAffected(t, osv, "", true)
320+
283321
// "LastAffected: 1" means all versions after this are not vulnerable
284322
osv = buildOSVWithAffected(
285323
database.Affected{
@@ -337,6 +375,40 @@ func TestOSV_IsAffected_AffectsWithEcosystem_SingleAffected(t *testing.T) {
337375

338376
// an empty version should always be treated as affected
339377
expectIsAffected(t, osv, "", true)
378+
379+
// mix of fixes, last_known_affected, and introduced, shuffled
380+
osv = buildOSVWithAffected(
381+
database.Affected{
382+
Package: database.Package{Ecosystem: lockfile.NpmEcosystem, Name: "my-package"},
383+
Ranges: []database.AffectsRange{
384+
buildEcosystemAffectsRange(
385+
database.RangeEvent{Introduced: "0"},
386+
database.RangeEvent{Introduced: "2.1.0"},
387+
database.RangeEvent{Fixed: "1"},
388+
database.RangeEvent{LastAffected: "3.1.9"},
389+
),
390+
},
391+
},
392+
)
393+
394+
for _, v := range []string{"0.0.0", "0.1.0", "0.0.0.1", "1.0.0-rc"} {
395+
expectIsAffected(t, osv, v, true)
396+
}
397+
398+
for _, v := range []string{"1.0.0", "1.1.0", "2.0.0rc2", "2.0.1"} {
399+
expectIsAffected(t, osv, v, false)
400+
}
401+
402+
for _, v := range []string{"2.1.1", "2.3.4", "3.0.0", "3.0.0-rc", "3.1.9"} {
403+
expectIsAffected(t, osv, v, true)
404+
}
405+
406+
for _, v := range []string{"3.2.0", "3.2.1", "4.0.0"} {
407+
expectIsAffected(t, osv, v, false)
408+
}
409+
410+
// an empty version should always be treated as affected
411+
expectIsAffected(t, osv, "", true)
340412
}
341413

342414
func TestOSV_IsAffected_AffectsWithEcosystem_MultipleAffected(t *testing.T) {
@@ -394,6 +466,95 @@ func TestOSV_IsAffected_AffectsWithEcosystem_MultipleAffected(t *testing.T) {
394466

395467
// an empty version should always be treated as affected
396468
expectIsAffected(t, osv, "", true)
469+
470+
// shuffled
471+
osv = buildOSVWithAffected(
472+
database.Affected{
473+
Package: database.Package{Ecosystem: lockfile.NpmEcosystem, Name: "my-package"},
474+
Ranges: []database.AffectsRange{
475+
buildEcosystemAffectsRange(
476+
database.RangeEvent{Fixed: "1"},
477+
database.RangeEvent{Introduced: "0"},
478+
),
479+
},
480+
},
481+
database.Affected{
482+
Package: database.Package{Ecosystem: lockfile.NpmEcosystem, Name: "my-package"},
483+
Ranges: []database.AffectsRange{
484+
buildEcosystemAffectsRange(
485+
database.RangeEvent{Fixed: "3.2.0"},
486+
database.RangeEvent{Introduced: "2.1.0"},
487+
),
488+
},
489+
},
490+
database.Affected{
491+
Package: database.Package{Ecosystem: lockfile.NpmEcosystem, Name: "my-package"},
492+
Ranges: []database.AffectsRange{
493+
buildEcosystemAffectsRange(
494+
database.RangeEvent{LastAffected: "3.5.0"},
495+
database.RangeEvent{Introduced: "3.3.0"},
496+
),
497+
},
498+
},
499+
)
500+
501+
for _, v := range []string{"0.0.0", "0.1.0", "0.0.0.1", "1.0.0-rc"} {
502+
expectIsAffected(t, osv, v, true)
503+
}
504+
505+
for _, v := range []string{"1.0.0", "1.1.0", "2.0.0rc2", "2.0.1"} {
506+
expectIsAffected(t, osv, v, false)
507+
}
508+
509+
for _, v := range []string{"2.1.1", "2.3.4", "3.0.0", "3.0.0-rc"} {
510+
expectIsAffected(t, osv, v, true)
511+
}
512+
513+
for _, v := range []string{"3.2.0", "3.2.1", "4.0.0"} {
514+
expectIsAffected(t, osv, v, false)
515+
}
516+
517+
for _, v := range []string{"3.3.1", "3.4.5"} {
518+
expectIsAffected(t, osv, v, true)
519+
}
520+
521+
// an empty version should always be treated as affected
522+
expectIsAffected(t, osv, "", true)
523+
524+
// zeros with build strings
525+
osv = buildOSVWithAffected(
526+
database.Affected{
527+
// golang.org/x/sys
528+
Package: database.Package{Ecosystem: lockfile.NpmEcosystem, Name: "my-package"},
529+
Ranges: []database.AffectsRange{
530+
buildEcosystemAffectsRange(
531+
database.RangeEvent{Fixed: "0.0.0-20220412211240-33da011f77ad"},
532+
database.RangeEvent{Introduced: "0"},
533+
),
534+
},
535+
},
536+
database.Affected{
537+
// golang.org/x/net
538+
Package: database.Package{Ecosystem: lockfile.NpmEcosystem, Name: "my-package"},
539+
Ranges: []database.AffectsRange{
540+
buildEcosystemAffectsRange(
541+
database.RangeEvent{Introduced: "0.0.0-20180925071336-cf3bd585ca2a"},
542+
database.RangeEvent{Fixed: "0"},
543+
),
544+
},
545+
},
546+
)
547+
548+
for _, v := range []string{"0.0.0", "0.14.0"} {
549+
expectIsAffected(t, osv, v, false)
550+
}
551+
552+
for _, v := range []string{"0.0.0-20180925071336-cf3bd585ca2a"} {
553+
expectIsAffected(t, osv, v, true)
554+
}
555+
556+
// an empty version should always be treated as affected
557+
expectIsAffected(t, osv, "", true)
397558
}
398559

399560
func TestOSV_IsAffected_AffectsWithEcosystem_PipNamesAreNormalised(t *testing.T) {
@@ -540,6 +701,40 @@ func TestOSV_IsAffected_AffectsWithSemver_SingleAffected(t *testing.T) {
540701
// an empty version should always be treated as affected
541702
expectIsAffected(t, osv, "", true)
542703

704+
// multiple fixes and introduced, shuffled
705+
osv = buildOSVWithAffected(
706+
database.Affected{
707+
Package: database.Package{Ecosystem: lockfile.NpmEcosystem, Name: "my-package"},
708+
Ranges: []database.AffectsRange{
709+
buildSemverAffectsRange(
710+
database.RangeEvent{Fixed: "1"},
711+
database.RangeEvent{Fixed: "3.2.0"},
712+
database.RangeEvent{Introduced: "0"},
713+
database.RangeEvent{Introduced: "2.1.0"},
714+
),
715+
},
716+
},
717+
)
718+
719+
for _, v := range []string{"0.0.0", "0.1.0", "0.0.0.1", "1.0.0-rc"} {
720+
expectIsAffected(t, osv, v, true)
721+
}
722+
723+
for _, v := range []string{"1.0.0", "1.1.0", "2.0.0rc2", "2.0.1"} {
724+
expectIsAffected(t, osv, v, false)
725+
}
726+
727+
for _, v := range []string{"2.1.1", "2.3.4", "3.0.0", "3.0.0-rc"} {
728+
expectIsAffected(t, osv, v, true)
729+
}
730+
731+
for _, v := range []string{"3.2.0", "3.2.1", "4.0.0"} {
732+
expectIsAffected(t, osv, v, false)
733+
}
734+
735+
// an empty version should always be treated as affected
736+
expectIsAffected(t, osv, "", true)
737+
543738
// "LastAffected: 1" means all versions after this are not vulnerable
544739
osv = buildOSVWithAffected(
545740
database.Affected{
@@ -594,6 +789,40 @@ func TestOSV_IsAffected_AffectsWithSemver_SingleAffected(t *testing.T) {
594789

595790
// an empty version should always be treated as affected
596791
expectIsAffected(t, osv, "", true)
792+
793+
// mix of fixes, last_known_affected, and introduced, shuffled
794+
osv = buildOSVWithAffected(
795+
database.Affected{
796+
Package: database.Package{Ecosystem: lockfile.NpmEcosystem, Name: "my-package"},
797+
Ranges: []database.AffectsRange{
798+
buildSemverAffectsRange(
799+
database.RangeEvent{Introduced: "2.1.0"},
800+
database.RangeEvent{Introduced: "0"},
801+
database.RangeEvent{LastAffected: "3.1.9"},
802+
database.RangeEvent{Fixed: "1"},
803+
),
804+
},
805+
},
806+
)
807+
808+
for _, v := range []string{"0.0.0", "0.1.0", "0.0.0.1", "1.0.0-rc"} {
809+
expectIsAffected(t, osv, v, true)
810+
}
811+
812+
for _, v := range []string{"1.0.0", "1.1.0", "2.0.0rc2", "2.0.1"} {
813+
expectIsAffected(t, osv, v, false)
814+
}
815+
816+
for _, v := range []string{"2.1.1", "2.3.4", "3.0.0", "3.0.0-rc"} {
817+
expectIsAffected(t, osv, v, true)
818+
}
819+
820+
for _, v := range []string{"3.2.0", "3.2.1", "4.0.0"} {
821+
expectIsAffected(t, osv, v, false)
822+
}
823+
824+
// an empty version should always be treated as affected
825+
expectIsAffected(t, osv, "", true)
597826
}
598827

599828
func TestOSV_IsAffected_AffectsWithSemver_MultipleAffected(t *testing.T) {

0 commit comments

Comments
 (0)