Skip to content

Commit 616fc86

Browse files
arpitjain099crozzy
authored andcommitted
ovalutil: guard a test element with no object reference
Both DefsToVulns loops read ObjectRef()[0] with no length check, while the StateRef() right below is guarded by len(stateRefs) > 0. The object reference is required by the OVAL schema but the parser does not enforce it, so a feed carrying an rpminfo_test or dpkginfo_test without an <object> child panics the updater with index out of range [0] with length 0 Skip the criterion instead, the same way an object lookup failure is already handled. Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
1 parent 69e467f commit 616fc86

3 files changed

Lines changed: 71 additions & 0 deletions

File tree

pkg/ovalutil/dpkg.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,11 @@ func DpkgDefsToVulns(ctx context.Context, root *oval.Root, protoVulns ProtoVulns
6464
//
6565
// thus we *should* only need to care about a single dpkginfo_object and optionally a state object providing the package's fixed-in version.
6666

67+
if len(objRefs) == 0 {
68+
stats.Obj++
69+
continue
70+
}
71+
6772
objRef := objRefs[0].ObjectRef
6873
object, err := dpkgObjectLookup(root, objRef)
6974
switch {

pkg/ovalutil/objectref_test.go

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
package ovalutil
2+
3+
import (
4+
"context"
5+
"encoding/xml"
6+
"fmt"
7+
"testing"
8+
9+
"github.com/quay/claircore"
10+
"github.com/quay/goval-parser/oval"
11+
)
12+
13+
// A test element is only required to carry an object reference; a feed that
14+
// omits it still parses.
15+
const missingObjectRef = `<oval_definitions>
16+
<definitions>
17+
<definition id="oval:com.example:def:1" class="patch">
18+
<metadata><title>example</title></metadata>
19+
<criteria>
20+
<criterion test_ref="oval:com.example:tst:1" comment="package is installed"/>
21+
</criteria>
22+
</definition>
23+
</definitions>
24+
<tests>
25+
<%s id="oval:com.example:tst:1" check="at least one" comment="package is installed"/>
26+
</tests>
27+
</oval_definitions>`
28+
29+
func protoVuln(oval.Definition) ([]*claircore.Vulnerability, error) {
30+
return []*claircore.Vulnerability{{Name: "example"}}, nil
31+
}
32+
33+
func TestDefsToVulnsWithoutObjectRef(t *testing.T) {
34+
for _, kind := range []string{"rpminfo_test", "dpkginfo_test"} {
35+
t.Run(kind, func(t *testing.T) {
36+
root := &oval.Root{}
37+
if err := xml.Unmarshal([]byte(fmt.Sprintf(missingObjectRef, kind)), root); err != nil {
38+
t.Fatalf("parsing the oval document: %v", err)
39+
}
40+
41+
var (
42+
vulns []*claircore.Vulnerability
43+
err error
44+
)
45+
switch kind {
46+
case "rpminfo_test":
47+
vulns, err = RPMDefsToVulns(context.Background(), root, protoVuln)
48+
case "dpkginfo_test":
49+
vulns, err = DpkgDefsToVulns(context.Background(), root, protoVuln, func(_ oval.Definition, name *oval.DpkgName) []string {
50+
return []string{name.Body}
51+
})
52+
}
53+
if err != nil {
54+
t.Fatalf("got an error: %v", err)
55+
}
56+
if len(vulns) != 0 {
57+
t.Errorf("expected the criterion to be skipped, got %d vulnerabilities", len(vulns))
58+
}
59+
})
60+
}
61+
}

pkg/ovalutil/rpm.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,11 @@ func RPMDefsToVulns(ctx context.Context, root *oval.Root, protoVulns ProtoVulnsF
8989
//
9090
// thus we *should* only need to care about a single rpminfo_object and optionally a state object providing the package's fixed-in version.
9191

92+
if len(objRefs) == 0 {
93+
slog.DebugContext(ctx, "test has no object reference, moving to next criterion", "test_ref", criterion.TestRef)
94+
continue
95+
}
96+
9297
objRef := objRefs[0].ObjectRef
9398
object, err := rpmObjectLookup(root, objRef)
9499
switch {

0 commit comments

Comments
 (0)