CVSS: panic and invalid vector validation

[pandatix]

Hey, quick review after the merge of #1143. Following information were automatically discovered through differential fuzzing with github.com/pandatix/go-cvss/differential.

CVSS v2.0

The implementation panics on valid inputs such as the ones containing Temporal metric E. For instance, the following stack trace is raised when parsing a valid CVSS v2.0 vector with this metric.

testing.go:1504: panic: invalid metric: E
            goroutine 29 [running]:
            runtime/debug.Stack()
                /usr/lib/go-1.21/src/runtime/debug/stack.go:24 +0x9b
            testing.tRunner.func1()
                /usr/lib/go-1.21/src/testing/testing.go:1504 +0x1ee
            panic({0x7fd000?, 0xc00009e950?})
                /usr/lib/go-1.21/src/runtime/panic.go:914 +0x21f
            github.com/quay/claircore/toolkit/types/cvss.(*V2).getScore(0x8801e9?, 0x6)
                /home/pandatix/go/pkg/mod/github.com/quay/claircore/[email protected]/types/cvss/cvss_v2.go:138 +0x2ac
            github.com/quay/claircore/toolkit/types/cvss.(*V2).Score(0xc000702688?)
                /home/pandatix/go/pkg/mod/github.com/quay/claircore/[email protected]/types/cvss/cvss_v2_score.go:36 +0xc5
            github.com/pandatix/go-cvss/differential_test.FuzzDifferential_V2_Claircore.func1(0xc00008bd40, {0xc0000ca151, 0x29})
                /home/pandatix/Documents/go-cvss/differential/v2_fuzz_test.go:189 +0x33f
            reflect.Value.call({0x802d00?, 0x882888?, 0x13?}, {0x86d323, 0x4}, {0xc0001e29f0, 0x2, 0x2?})
                /usr/lib/go-1.21/src/reflect/value.go:596 +0xce7
            reflect.Value.Call({0x802d00?, 0x882888?, 0xac9800?}, {0xc0001e29f0?, 0x86c860?, 0x5697ad?})
                /usr/lib/go-1.21/src/reflect/value.go:380 +0xb9
            testing.(*F).Fuzz.func1.1(0x409800?)
                /usr/lib/go-1.21/src/testing/fuzz.go:335 +0x347
            testing.tRunner(0xc00008bd40, 0xc0001f6480)
                /usr/lib/go-1.21/src/testing/testing.go:1595 +0xff
            created by testing.(*F).Fuzz.func1 in goroutine 18
                /usr/lib/go-1.21/src/testing/fuzz.go:322 +0x597

Given the FIRST.ORG CVSS v2 documentation this should be considered valid vector. As it has not been released (in main but without a tag yet) I don't think it require a CVE despite it should still be better to make sure people are aware when a fix is made available.

CVSS v3

The current implementation does not return any error on invalid vectors such as AV:N/AC:L/PR:N/UI:N/S:C/C:H/I:H/A:H. As of the FIRST.ORG CVSS v3.1 specification (statement is also valid with v3.0) this is an invalid vector string as it does not contain the mandatory prefix CVSS:3.1.

CVSS v4

As for the previous, according to the FIRST.ORG CVSS v4.0 specification invalid vectors as CVSS:/AV:A/AC:L/AT:N/PR:H/UI:A/VC:L/VI:L/VA:N/SC:N/SA:N/S:X does not return any error despite an invalid vector prefix.

[hdonnay]

Sent #1232 for review :+1:

[pandatix]

Hey, quick update on the latest implem that is supposed to close this issue.

CVSS v2

Vector AV:A/AC:L/Au:N/C:C/I:C/A:C/CDP:H/TD:H/CR:H is parsed without returning an error despite it being invalid (missing IR and AR metrics from the environmental metrics group, mandatory once there is at least one metric of the same group).

CVSS v3

Seems fine :grin:

CVSS v4

Vector CVSS:4.0/AV:A/AC:L/AT:N/PR:H/UI:A/VC:LN/VI:L/VA:N/SC:N/SI:N/SA:N is parsed without returning an error despite being invalid (metric VC has invalid value LN, most probably the concatenation of the two valid values L and N).