Skip to content

Commit 27429d7

Browse files
Correct a bug with AddPkgInfo commit event assembly (#2280)
While investigating an unexpected false positive for CVE-2019-14248 it was discovered that commit event ranges were being incorrectly assembled, and all of the events were being inserted into a single element, rather than one per element, i.e. ``` { "ranges": [ { "type": "GIT", "repo": "https://github.com/netwide-assembler/nasm", "events": [ { "introduced": "9a1216a1efa0ccb48e5df97acc763ea3de71e0ce", "last_affected": "74246c499ea4313fb8837977dc0c135fc50567c0" } ] } ] } ``` instead of: ``` { "ranges": [ { "type": "GIT", "repo": "https://github.com/netwide-assembler/nasm", "events": [ { "introduced": "9a1216a1efa0ccb48e5df97acc763ea3de71e0ce", }, { "last_affected": "74246c499ea4313fb8837977dc0c135fc50567c0" } ] } ] } ``` which produced an invalid OSV record.
1 parent 88257af commit 27429d7

File tree

3 files changed

+82
-17
lines changed

3 files changed

+82
-17
lines changed

vulnfeeds/cves/versions.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,15 @@ func (vi *VersionInfo) HasLastAffectedCommits(repo string) bool {
150150
return false
151151
}
152152

153+
func (vi *VersionInfo) HasLimitCommits(repo string) bool {
154+
for _, ac := range vi.AffectedCommits {
155+
if strings.EqualFold(ac.Repo, repo) && ac.Limit != "" {
156+
return true
157+
}
158+
}
159+
return false
160+
}
161+
153162
func (vi *VersionInfo) FixedCommits(repo string) (FixedCommits []string) {
154163
for _, ac := range vi.AffectedCommits {
155164
if strings.EqualFold(ac.Repo, repo) && ac.Fixed != "" {

vulnfeeds/vulns/vulns.go

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -249,14 +249,18 @@ func (v *Vulnerability) AddPkgInfo(pkgInfo PackageInfo) {
249249
hasAddedZeroIntroduced[ac.Repo] = true
250250
}
251251

252-
entry.Events = append(entry.Events,
253-
Event{
254-
Introduced: ac.Introduced,
255-
Fixed: ac.Fixed,
256-
LastAffected: ac.LastAffected,
257-
Limit: ac.Limit,
258-
},
259-
)
252+
if pkgInfo.VersionInfo.HasIntroducedCommits(ac.Repo) {
253+
entry.Events = append(entry.Events, Event{Introduced: ac.Introduced})
254+
}
255+
if pkgInfo.VersionInfo.HasFixedCommits(ac.Repo) {
256+
entry.Events = append(entry.Events, Event{Fixed: ac.Fixed})
257+
}
258+
if pkgInfo.VersionInfo.HasLastAffectedCommits(ac.Repo) {
259+
entry.Events = append(entry.Events, Event{LastAffected: ac.LastAffected})
260+
}
261+
if pkgInfo.VersionInfo.HasLimitCommits(ac.Repo) {
262+
entry.Events = append(entry.Events, Event{Limit: ac.Limit})
263+
}
260264
gitCommitRangesByRepo[ac.Repo] = entry
261265
}
262266

@@ -274,12 +278,20 @@ func (v *Vulnerability) AddPkgInfo(pkgInfo PackageInfo) {
274278
for _, av := range pkgInfo.VersionInfo.AffectedVersions {
275279
if av.Introduced != "" {
276280
hasIntroduced = true
281+
versionRange.Events = append(versionRange.Events, Event{
282+
Introduced: av.Introduced,
283+
})
284+
}
285+
if av.Fixed != "" {
286+
versionRange.Events = append(versionRange.Events, Event{
287+
Fixed: av.Fixed,
288+
})
289+
}
290+
if av.LastAffected != "" {
291+
versionRange.Events = append(versionRange.Events, Event{
292+
LastAffected: av.LastAffected,
293+
})
277294
}
278-
versionRange.Events = append(versionRange.Events, Event{
279-
Introduced: av.Introduced,
280-
Fixed: av.Fixed,
281-
LastAffected: av.LastAffected,
282-
})
283295
}
284296

285297
if !hasIntroduced {

vulnfeeds/vulns/vulns_test.go

Lines changed: 48 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -192,10 +192,37 @@ func TestAddPkgInfo(t *testing.T) {
192192
},
193193
},
194194
}
195-
vuln.AddPkgInfo(testPkgInfoNameEco)
196-
vuln.AddPkgInfo(testPkgInfoPURL)
197-
vuln.AddPkgInfo(testPkgInfoCommits)
198-
vuln.AddPkgInfo(testPkgInfoHybrid)
195+
testPkgInfoCommitsMultiple := PackageInfo{
196+
VersionInfo: cves.VersionInfo{
197+
AffectedCommits: []cves.AffectedCommit{
198+
{
199+
Introduced: "0xdeadbeef",
200+
Fixed: "dsafwefwfe370a9e65d68d62ef37345597e4100b0e87021dfb",
201+
Repo: "github.com/foo/bar",
202+
},
203+
},
204+
},
205+
}
206+
testPkgInfoEcoMultiple := PackageInfo{
207+
PkgName: "TestNameWithIntroduced",
208+
Ecosystem: "TestEco",
209+
VersionInfo: cves.VersionInfo{
210+
AffectedVersions: []cves.AffectedVersion{
211+
{
212+
Introduced: "1.0.0-1",
213+
Fixed: "1.2.3-4",
214+
},
215+
},
216+
},
217+
}
218+
vuln.AddPkgInfo(testPkgInfoNameEco) // This will end up in vuln.Affected[0]
219+
vuln.AddPkgInfo(testPkgInfoPURL) // This will end up in vuln.Affected[1]
220+
vuln.AddPkgInfo(testPkgInfoCommits) // This will end up in vuln.Affected[2]
221+
vuln.AddPkgInfo(testPkgInfoHybrid) // This will end up in vuln.Affected[3]
222+
vuln.AddPkgInfo(testPkgInfoCommitsMultiple) // This will end up in vuln.Affected[4]
223+
vuln.AddPkgInfo(testPkgInfoEcoMultiple) // This will end up in vuln.Affected[5]
224+
225+
t.Logf("Resulting vuln: %+v", vuln)
199226

200227
// testPkgInfoNameEco vvvvvvvvvvvvvvv
201228
if vuln.Affected[0].Package.Name != testPkgInfoNameEco.PkgName {
@@ -213,6 +240,10 @@ func TestAddPkgInfo(t *testing.T) {
213240
if vuln.Affected[0].Ranges[0].Events[1].Fixed != testPkgInfoNameEco.VersionInfo.AffectedVersions[0].Fixed {
214241
t.Errorf("AddPkgInfo has not correctly added ranges fixed.")
215242
}
243+
244+
if vuln.Affected[0].Ranges[0].Events[0].Introduced != "0" {
245+
t.Errorf("AddPkgInfo has not correctly added zero introduced commit.")
246+
}
216247
// testPkgInfoNameEco ^^^^^^^^^^^^^^^
217248

218249
// testPkgInfoPURL vvvvvvvvvvvvvvv
@@ -251,6 +282,19 @@ func TestAddPkgInfo(t *testing.T) {
251282
}
252283
// testPkgInfoCommits ^^^^^^^^^^^^^^^
253284

285+
// testPkgInfoCommitsMultiple vvvvvvvvvvvvv
286+
if len(vuln.Affected[4].Ranges[0].Events) != 2 {
287+
t.Errorf("AddPkgInfo has not correctly added distinct range events from commits: %+v", vuln.Affected[4].Ranges)
288+
}
289+
// testPkgInfoCommitsMultiple ^^^^^^^^^^^^^
290+
291+
// testPkgInfoEcoMultiple vvvvvvvvvvvvv
292+
if len(vuln.Affected[5].Ranges[0].Events) != 2 {
293+
t.Errorf("AddPkgInfo has not correctly added distinct range events from versions: %+v", vuln.Affected[5].Ranges)
294+
}
295+
// testPkgInfoEcoMultiple ^^^^^^^^^^^^^
296+
297+
254298
for _, a := range vuln.Affected {
255299
perRepoZeroIntroducedCommitHashCount := make(map[string]int)
256300
for _, r := range a.Ranges {

0 commit comments

Comments
 (0)