From 068edb66d4f4fcde36f6c6ca407615d061daf4ec Mon Sep 17 00:00:00 2001
From: Hank Donnay <hdonnay@redhat.com>
Date: Wed, 31 Jan 2024 13:02:25 -0600
Subject: [PATCH] cvss: fix v2 group-wise marshaling behavior

Section 2.4 of the standard implies that a vector should have metrics
with not defined values included if a metric in the group has a defined
value.

One might also say that the environmental group implies the temporal
group, but that's quite annoying and the standard should say that if it
wants to say that.

Signed-off-by: Hank Donnay <hdonnay@redhat.com>
---
 toolkit/types/cvss/cvss.go         | 45 ++++++++++++++++++++----------
 toolkit/types/cvss/cvss_v2.go      |  5 +++-
 toolkit/types/cvss/cvss_v2_test.go | 12 ++++----
 3 files changed, 41 insertions(+), 21 deletions(-)

diff --git a/toolkit/types/cvss/cvss.go b/toolkit/types/cvss/cvss.go
index 88bd9d923..7a6333242 100644
--- a/toolkit/types/cvss/cvss.go
+++ b/toolkit/types/cvss/cvss.go
@@ -121,21 +121,38 @@ func mkRevLookup[M Metric]() map[string]M {
 //
 // The [Vector.getString] method is used here.
 func marshalVector[M Metric, V Vector[M]](prefix string, v V) ([]byte, error) {
-	text := append(make([]byte, 0, 64), prefix...)
-	for i := 0; i < M(0).num(); i++ {
-		m := M(i)
-		val, err := v.getString(m)
-		switch {
-		case errors.Is(err, nil):
-		case errors.Is(err, errValueUnset):
-			continue
-		default:
-			return nil, errors.New("invalid cvss vector")
+	text := append(make([]byte, 0, 64), prefix...) // Guess at an initial capacity.
+	var err error
+	// This is a rangefunc-style iterator.
+	v.groups(func(b [2]int) bool {
+		var set bool
+		orig := len(text)
+		for i := b[0]; i < b[1]; i++ {
+			m := M(i)
+			val, err := v.getString(m)
+			switch {
+			case errors.Is(err, nil):
+				set = true
+			case errors.Is(err, errValueUnset) && val == "":
+				continue
+			case errors.Is(err, errValueUnset):
+			default:
+				err = errors.New("invalid cvss vector")
+				return false
+			}
+
+			text = append(text, '/')
+			text = append(text, m.String()...)
+			text = append(text, ':')
+			text = append(text, val...)
+		}
+		if !set {
+			text = text[:orig]
 		}
-		text = append(text, '/')
-		text = append(text, m.String()...)
-		text = append(text, ':')
-		text = append(text, val...)
+		return true
+	})
+	if err != nil {
+		return nil, err
 	}
 	// v2 hack
 	if prefix == "" {
diff --git a/toolkit/types/cvss/cvss_v2.go b/toolkit/types/cvss/cvss_v2.go
index 3db6840ee..0b4a32521 100644
--- a/toolkit/types/cvss/cvss_v2.go
+++ b/toolkit/types/cvss/cvss_v2.go
@@ -114,8 +114,11 @@ func (v *V2) String() string {
 // GetString implements [Vector].
 func (v *V2) getString(m V2Metric) (string, error) {
 	b := v.mv[int(m)]
-	if b == 0 {
+	switch {
+	case b == 0 && m <= V2Availability:
 		return "", errValueUnset
+	case b == 0:
+		return "ND", errValueUnset
 	}
 	return v2Unparse(m, b), nil
 }
diff --git a/toolkit/types/cvss/cvss_v2_test.go b/toolkit/types/cvss/cvss_v2_test.go
index 47ac73388..beb6ec01c 100644
--- a/toolkit/types/cvss/cvss_v2_test.go
+++ b/toolkit/types/cvss/cvss_v2_test.go
@@ -19,12 +19,12 @@ func TestV2(t *testing.T) {
 	})
 	t.Run("Roundtrip", func(t *testing.T) {
 		vecs := []string{
-			"AV:N/AC:L/Au:N/C:N/I:N/A:C",                                          // CVE-2002-0392
-			"AV:N/AC:L/Au:N/C:C/I:C/A:C",                                          // CVE-2003-0818
-			"AV:L/AC:H/Au:N/C:C/I:C/A:C",                                          // CVE-2003-0062
-			"AV:L/AC:H/Au:N/C:C/I:C/A:C/E:F/RL:OF/RC:C/CDP:H/TD:H/CR:M/IR:M/AR:H", // CVE-2002-0392
-			"AV:L/AC:H/Au:N/C:C/I:C/A:C/E:F/RL:OF/RC:UR/CDP:ND/TD:ND",             // made up
-			"AV:L/AC:H/Au:N/C:C/I:C/A:C/E:F/RL:OF/RC:UR/CDP:LM/TD:ND",             // made up
+			"AV:N/AC:L/Au:N/C:N/I:N/A:C",                                                // CVE-2002-0392
+			"AV:N/AC:L/Au:N/C:C/I:C/A:C",                                                // CVE-2003-0818
+			"AV:L/AC:H/Au:N/C:C/I:C/A:C",                                                // CVE-2003-0062
+			"AV:L/AC:H/Au:N/C:C/I:C/A:C/E:F/RL:OF/RC:C/CDP:H/TD:H/CR:M/IR:M/AR:H",       // CVE-2002-0392
+			"AV:L/AC:H/Au:N/C:C/I:C/A:C/E:F/RL:OF/RC:UR/CDP:ND/TD:ND/CR:ND/IR:ND/AR:ND", // made up
+			"AV:L/AC:H/Au:N/C:C/I:C/A:C/E:F/RL:OF/RC:UR/CDP:LM/TD:ND/CR:ND/IR:ND/AR:ND", // made up
 		}
 		Roundtrip[V2, V2Metric, *V2](t, vecs)
 	})