From 7584ce6c236a31b5c989245d92ca3b9a1d1da4d7 Mon Sep 17 00:00:00 2001 From: Hank Donnay Date: Fri, 26 Jul 2024 16:46:10 -0500 Subject: [PATCH] cpe: add error handling for invalid URI inputs Some previously-allowed URI-bound CPEs will now report errors. Tests have been added (and modified) to this effect. Signed-off-by: Hank Donnay --- toolkit/types/cpe/match_test.go | 2 +- toolkit/types/cpe/unbind.go | 35 +++- toolkit/types/cpe/wfn_test.go | 353 +++++++++++++++++--------------- 3 files changed, 214 insertions(+), 176 deletions(-) diff --git a/toolkit/types/cpe/match_test.go b/toolkit/types/cpe/match_test.go index 2951af9ad..7ea3cc257 100644 --- a/toolkit/types/cpe/match_test.go +++ b/toolkit/types/cpe/match_test.go @@ -22,7 +22,7 @@ func TestMatch(t *testing.T) { // There seems to be no test vectors for the match specification. matchTable := []testcase{ { - Source: `cpe:/a:Adobe::9.*::PalmOS`, + Source: `cpe:/a:Adobe::9.%02::PalmOS`, Target: `cpe:/a::Reader:9.3.2:-:-`, Want: Relations([NumAttr]Relation{ Equal, Subset, Superset, Superset, Superset, Disjoint, Equal, Equal, Equal, Equal, Equal, diff --git a/toolkit/types/cpe/unbind.go b/toolkit/types/cpe/unbind.go index dbf38c831..0139b4e00 100644 --- a/toolkit/types/cpe/unbind.go +++ b/toolkit/types/cpe/unbind.go @@ -64,16 +64,25 @@ func UnbindURI(s string) (WFN, error) { if i == 5 && strings.HasPrefix(c, "~") { attrs := [...]Attribute{Edition, SwEdition, TargetSW, TargetHW, Other} for i, c := range strings.SplitN(c, `~`, 6)[1:] { - r.Attr[attrs[i]].unbindURI(&b, c) + if err := r.Attr[attrs[i]].unbindURI(&b, c); err != nil { + return WFN{}, fmt.Errorf("cpe: %v: %w", attrs[i], err) + } } continue } - r.Attr[attrs[i]].unbindURI(&b, c) + if err := r.Attr[attrs[i]].unbindURI(&b, c); err != nil { + return WFN{}, fmt.Errorf("cpe: %v: %w", attrs[i], err) + } } return r, r.Valid() } -func (v *Value) unbindURI(b *strings.Builder, s string) { +func (v *Value) unbindURI(b *strings.Builder, s string) error { + // From the characters that should be escaped, see + // https://nvlpubs.nist.gov/nistpubs/Legacy/IR/nistir7695.pdf table 6-1. + // Need to allow '%' and '~'. The former for escapes and the latter because + // they're allowed in CPE2.2 URIs. + const disallow = "!\"#$&'()*+,/:;<=>?@[\\]^`{|}" b.Reset() switch s { case ``: @@ -82,15 +91,29 @@ func (v *Value) unbindURI(b *strings.Builder, s string) { v.Kind = ValueNA default: v.Kind = ValueSet - valueURI.WriteString(b, strings.ToLower(s)) + s = strings.ToLower(s) + if i := strings.IndexAny(s, disallow); i != -1 { + return fmt.Errorf("disallowed character %q", s[i]) + } + valueURI.WriteString(b, s) v.V = b.String() } + return nil } -// ValueURI is a replace that undoes the URI percent encoding. +// ValueURI is a replacer that undoes URI percent encoding and puts legally URI +// encoded characters into formatted-string escaping. +// +// If there are remaining percent encodes, they will be passed through. In +// theory we could normalize these, but I think we'd need to use something a bit +// more heavy-duty, like a [text.Transformer]. var valueURI = strings.NewReplacer( `.`, `\.`, `-`, `\-`, + // 2.2 CPEs don't have any special handling for tilde, but 2.3 puts special + // semantics in the "Edition" component. Those are handled farther up in the + // call stack, so handle the corner case where there's a tile in another + // component. `~`, `\~`, // The specified algorithm sticks validation logic for * and ? in the // unquoting. We skip that and just make sure to validate later. @@ -125,6 +148,8 @@ var valueURI = strings.NewReplacer( `%7c`, `\|`, `%7d`, `\}`, `%7e`, `\~`, + // Do not handle: + //`:`, `%3a`, // Can't be here anyway, it's used to separate components. ) // UnbindFS attempts to unbind a string as CPE 2.3 formatted string into a WFN. diff --git a/toolkit/types/cpe/wfn_test.go b/toolkit/types/cpe/wfn_test.go index ee8d21364..961a4b067 100644 --- a/toolkit/types/cpe/wfn_test.go +++ b/toolkit/types/cpe/wfn_test.go @@ -9,6 +9,10 @@ import ( "github.com/google/go-cmp/cmp" ) +func valueAny() Value { return Value{Kind: ValueAny} } +func valueNA() Value { return Value{Kind: ValueNA} } +func valueSet(v string) Value { return Value{Kind: ValueSet, V: v} } + func TestValidate(t *testing.T) { t.Parallel() tt := []struct { @@ -91,81 +95,81 @@ func TestBinding(t *testing.T) { // wfn:[part="a",vendor="microsoft",product="internet_explorer",version="8\.0\.6001",update="beta",edition=ANY] { WFN: WFN{Attr: [NumAttr]Value{ - {Kind: ValueSet, V: "a"}, - {Kind: ValueSet, V: "microsoft"}, - {Kind: ValueSet, V: "internet_explorer"}, - {Kind: ValueSet, V: "8\\.0\\.6001"}, - {Kind: ValueSet, V: "beta"}, - {Kind: ValueAny}, + valueSet("a"), + valueSet("microsoft"), + valueSet("internet_explorer"), + valueSet(`8\.0\.6001`), + valueSet("beta"), + valueAny(), }}, Bound: `cpe:2.3:a:microsoft:internet_explorer:8.0.6001:beta:*:*:*:*:*:*`, }, // wfn:[part="a",vendor="microsoft",product="internet_explorer",version="8\.*",update="sp?",edition=ANY] { WFN: WFN{Attr: [NumAttr]Value{ - {Kind: ValueSet, V: "a"}, - {Kind: ValueSet, V: "microsoft"}, - {Kind: ValueSet, V: "internet_explorer"}, - {Kind: ValueSet, V: "8\\.*"}, - {Kind: ValueSet, V: "sp?"}, + valueSet("a"), + valueSet("microsoft"), + valueSet("internet_explorer"), + valueSet(`8\.*`), + valueSet("sp?"), }}, Bound: `cpe:2.3:a:microsoft:internet_explorer:8.*:sp?:*:*:*:*:*:*`, }, // wfn:[part="a",vendor="microsoft",product="internet_explorer",version="8\.\*",update="sp?"] { WFN: WFN{Attr: [NumAttr]Value{ - {Kind: ValueSet, V: "a"}, - {Kind: ValueSet, V: "microsoft"}, - {Kind: ValueSet, V: "internet_explorer"}, - {Kind: ValueSet, V: "8\\.\\*"}, - {Kind: ValueSet, V: "sp?"}, + valueSet("a"), + valueSet("microsoft"), + valueSet("internet_explorer"), + valueSet(`8\.\*`), + valueSet("sp?"), }}, Bound: `cpe:2.3:a:microsoft:internet_explorer:8.\*:sp?:*:*:*:*:*:*`, }, // wfn:[part="a",vendor="hp",product="insight",version="7\.4\.0\.1570",update=NA,sw_edition="online",target_sw="win2003",target_hw="x64"] { WFN: WFN{Attr: [NumAttr]Value{ - {Kind: ValueSet, V: "a"}, - {Kind: ValueSet, V: "hp"}, - {Kind: ValueSet, V: "insight"}, - {Kind: ValueSet, V: `7\.4\.0\.1570`}, - {Kind: ValueNA}, + valueSet("a"), + valueSet("hp"), + valueSet("insight"), + valueSet(`7\.4\.0\.1570`), + valueNA(), {}, {}, - {Kind: ValueSet, V: "online"}, - {Kind: ValueSet, V: "win2003"}, - {Kind: ValueSet, V: "x64"}, + valueSet("online"), + valueSet("win2003"), + valueSet("x64"), }}, Bound: `cpe:2.3:a:hp:insight:7.4.0.1570:-:*:*:online:win2003:x64:*`, }, // wfn:[part="a",vendor="hp",product="openview_network_manager",version="7\.51",target_sw="linux"] { WFN: WFN{Attr: [NumAttr]Value{ - {Kind: ValueSet, V: "a"}, - {Kind: ValueSet, V: "hp"}, - {Kind: ValueSet, V: "openview_network_manager"}, - {Kind: ValueSet, V: `7\.51`}, + valueSet("a"), + valueSet("hp"), + valueSet("openview_network_manager"), + valueSet(`7\.51`), {}, {}, {}, {}, - {Kind: ValueSet, V: "linux"}, + valueSet("linux"), }}, Bound: `cpe:2.3:a:hp:openview_network_manager:7.51:*:*:*:*:linux:*:*`, }, // wfn:[part="a",vendor="foo\\bar",product="big\$money_2010",sw_edition="special",target_sw="ipod_touch",target_hw="80gb"] { WFN: WFN{Attr: [NumAttr]Value{ - {Kind: ValueSet, V: "a"}, - {Kind: ValueSet, V: `foo\\bar`}, - {Kind: ValueSet, V: `big\$money_2010`}, + valueSet("a"), + valueSet(`foo\\bar`), + valueSet(`big\$money_2010`), {}, {}, {}, {}, - {Kind: ValueSet, V: "special"}, - {Kind: ValueSet, V: "ipod_touch"}, - {Kind: ValueSet, V: "80gb"}, + valueSet("special"), + valueSet("ipod_touch"), + valueSet("80gb"), }}, Bound: `cpe:2.3:a:foo\\bar:big\$money_2010:*:*:*:*:special:ipod_touch:80gb:*`, }, @@ -180,48 +184,72 @@ func TestBinding(t *testing.T) { func TestUnbinding(t *testing.T) { t.Parallel() - // UnbindTable is a table of string to WFN mappings copied out of the standards + + type unbindCase struct { + Bound string + WFN WFN + Error bool + } + inner := func(tcs []unbindCase, unbind func(string) (WFN, error)) func(*testing.T) { + return func(t *testing.T) { + t.Helper() + for i, tc := range tcs { + t.Logf("%02d: %q", i, tc.Bound) + got, err := unbind(tc.Bound) + if tc.Error { + if err == nil { + t.Errorf("%02d: expected error, got nil", i) + } + continue + } + if err != nil { + t.Logf("%02d: %v", i, err) + } + if want := tc.WFN; !cmp.Equal(got, want) { + t.Errorf("%02d: %v", i, cmp.Diff(got, want)) + } + } + } + } + + // FsTable is a table of string to WFN mappings copied out of the standards // document: https://nvlpubs.nist.gov/nistpubs/Legacy/IR/nistir7695.pdf // // The goal wfn from the text is kept in a comment, then transcribed by hand into a // WFN literal, and the starting binding is copied verbatim. - unbindTable := []struct { - Bound string - WFN WFN - Error bool - }{ + fsTable := []unbindCase{ // wfn:[part="a",vendor="microsoft",product="internet_explorer",version="8\.0\.6001",update="beta",edition=ANY,language=ANY,sw_edition=ANY,target_sw=ANY,target_hw=ANY,other=ANY] { Bound: `cpe:2.3:a:microsoft:internet_explorer:8.0.6001:beta:*:*:*:*:*:*`, WFN: WFN{Attr: [NumAttr]Value{ - {Kind: ValueSet, V: "a"}, - {Kind: ValueSet, V: "microsoft"}, - {Kind: ValueSet, V: "internet_explorer"}, - {Kind: ValueSet, V: "8\\.0\\.6001"}, - {Kind: ValueSet, V: "beta"}, - {Kind: ValueAny}, - {Kind: ValueAny}, - {Kind: ValueAny}, - {Kind: ValueAny}, - {Kind: ValueAny}, - {Kind: ValueAny}, + valueSet("a"), + valueSet("microsoft"), + valueSet("internet_explorer"), + valueSet(`8\.0\.6001`), + valueSet("beta"), + valueAny(), + valueAny(), + valueAny(), + valueAny(), + valueAny(), + valueAny(), }}, }, // wfn:[part="a",vendor="hp",product="insight_diagnostics",version="7\.4\.0\.1570",update=NA,edition=ANY,language=ANY,sw_edition="online",target_sw="win2003",target_hw="x64",other=ANY] { Bound: `cpe:2.3:a:hp:insight_diagnostics:7.4.0.1570:-:*:*:online:win2003:x64:*`, WFN: WFN{Attr: [NumAttr]Value{ - {Kind: ValueSet, V: "a"}, - {Kind: ValueSet, V: "hp"}, - {Kind: ValueSet, V: "insight_diagnostics"}, - {Kind: ValueSet, V: `7\.4\.0\.1570`}, - {Kind: ValueNA}, - {Kind: ValueAny}, - {Kind: ValueAny}, - {Kind: ValueSet, V: "online"}, - {Kind: ValueSet, V: "win2003"}, - {Kind: ValueSet, V: "x64"}, - {Kind: ValueAny}, + valueSet("a"), + valueSet("hp"), + valueSet("insight_diagnostics"), + valueSet(`7\.4\.0\.1570`), + valueNA(), + valueAny(), + valueAny(), + valueSet("online"), + valueSet("win2003"), + valueSet("x64"), + valueAny(), }}, }, // Invalid bound form because of unquoted (and misplaced) asterisk. @@ -233,117 +261,93 @@ func TestUnbinding(t *testing.T) { { Bound: `cpe:2.3:a:foo\\bar:big\$money:2010:*:*:*:special:ipod_touch:80gb:*`, WFN: WFN{Attr: [NumAttr]Value{ - {Kind: ValueSet, V: "a"}, - {Kind: ValueSet, V: `foo\\bar`}, - {Kind: ValueSet, V: `big\$money`}, - {Kind: ValueSet, V: "2010"}, - {Kind: ValueAny}, - {Kind: ValueAny}, - {Kind: ValueAny}, - {Kind: ValueSet, V: "special"}, - {Kind: ValueSet, V: "ipod_touch"}, - {Kind: ValueSet, V: "80gb"}, - {Kind: ValueAny}, + valueSet("a"), + valueSet(`foo\\bar`), + valueSet(`big\$money`), + valueSet("2010"), + valueAny(), + valueAny(), + valueAny(), + valueSet("special"), + valueSet("ipod_touch"), + valueSet("80gb"), + valueAny(), }}, }, } - for _, tc := range unbindTable { - t.Logf("%q", tc.Bound) - got, err := UnbindFS(tc.Bound) - if tc.Error { - t.Log(err) - if err == nil { - t.Error("expected error, got nil") - } - continue - } - if err != nil { - t.Error(err) - } - if want := tc.WFN; !cmp.Equal(got, want) { - t.Error(cmp.Diff(got, want)) - } - } -} -func TestURIUnbinding(t *testing.T) { - t.Parallel() // This table is made from the URI unbinding examples in the standards document. - tt := []struct { - Bound string - WFN WFN - Error bool - }{ + uriTable := []unbindCase{ // wfn:[part="a",vendor="microsoft",product="internet_explorer",version="8\.0\.6001",update="beta",edition=ANY,language=ANY] { WFN: WFN{Attr: [NumAttr]Value{ - {Kind: ValueSet, V: "a"}, - {Kind: ValueSet, V: "microsoft"}, - {Kind: ValueSet, V: "internet_explorer"}, - {Kind: ValueSet, V: `8\.0\.6001`}, - {Kind: ValueSet, V: "beta"}, - {Kind: ValueAny}, - {Kind: ValueAny}, + valueSet("a"), + valueSet("microsoft"), + valueSet("internet_explorer"), + valueSet(`8\.0\.6001`), + valueSet("beta"), + valueAny(), + valueAny(), }}, Bound: `cpe:/a:microsoft:internet_explorer:8.0.6001:beta`, }, // wfn:[part="a",vendor="microsoft",product="internet_explorer",version="8\.\*",update="sp\?",edition=ANY,language=ANY] { WFN: WFN{Attr: [NumAttr]Value{ - {Kind: ValueSet, V: "a"}, - {Kind: ValueSet, V: "microsoft"}, - {Kind: ValueSet, V: "internet_explorer"}, - {Kind: ValueSet, V: `8\.\*`}, - {Kind: ValueSet, V: `sp\?`}, - {Kind: ValueAny}, - {Kind: ValueAny}, + valueSet("a"), + valueSet("microsoft"), + valueSet("internet_explorer"), + valueSet(`8\.\*`), + valueSet(`sp\?`), + valueAny(), + valueAny(), }}, Bound: `cpe:/a:microsoft:internet_explorer:8.%2a:sp%3f`, }, // wfn:[part="a",vendor="microsoft",product="internet_explorer",version="8\.*",update="sp?",edition=ANY,language=ANY] { WFN: WFN{Attr: [NumAttr]Value{ - {Kind: ValueSet, V: "a"}, - {Kind: ValueSet, V: "microsoft"}, - {Kind: ValueSet, V: "internet_explorer"}, - {Kind: ValueSet, V: `8\.*`}, - {Kind: ValueSet, V: "sp?"}, - {Kind: ValueAny}, - {Kind: ValueAny}, + valueSet("a"), + valueSet("microsoft"), + valueSet("internet_explorer"), + valueSet(`8\.*`), + valueSet("sp?"), + valueAny(), + valueAny(), }}, Bound: `cpe:/a:microsoft:internet_explorer:8.%02:sp%01`, }, // wfn:[part="a",vendor="hp",product="insight_diagnostics",version="7\.4\.0\.1570",update=ANY,edition=ANY,sw_edition="online",target_sw="win2003",target_hw="x64",other=ANY,language=ANY] { WFN: WFN{Attr: [NumAttr]Value{ - {Kind: ValueSet, V: "a"}, - {Kind: ValueSet, V: "hp"}, - {Kind: ValueSet, V: "insight_diagnostics"}, - {Kind: ValueSet, V: `7\.4\.0\.1570`}, - {Kind: ValueAny}, - {Kind: ValueAny}, - {Kind: ValueAny}, - {Kind: ValueSet, V: "online"}, - {Kind: ValueSet, V: "win2003"}, - {Kind: ValueSet, V: "x64"}, - {Kind: ValueAny}, + valueSet("a"), + valueSet("hp"), + valueSet("insight_diagnostics"), + valueSet(`7\.4\.0\.1570`), + valueAny(), + valueAny(), + valueAny(), + valueSet("online"), + valueSet("win2003"), + valueSet("x64"), + valueAny(), }}, Bound: `cpe:/a:hp:insight_diagnostics:7.4.0.1570::~~online~win2003~x64~`, }, // wfn:[part="a",vendor="hp",product="openview_network_manager",version="7\.51",update=NA,edition=ANY,sw_edition=ANY,target_sw="linux",target_HW=ANY,other=ANY,language=ANY] { WFN: WFN{Attr: [NumAttr]Value{ - {Kind: ValueSet, V: "a"}, - {Kind: ValueSet, V: "hp"}, - {Kind: ValueSet, V: "openview_network_manager"}, - {Kind: ValueSet, V: `7\.51`}, - {Kind: ValueNA}, - {Kind: ValueAny}, - {Kind: ValueAny}, - {Kind: ValueAny}, - {Kind: ValueSet, V: "linux"}, - {Kind: ValueAny}, - {Kind: ValueAny}, + valueSet("a"), + valueSet("hp"), + valueSet("openview_network_manager"), + valueSet(`7\.51`), + valueNA(), + valueAny(), + valueAny(), + valueAny(), + valueSet("linux"), + valueAny(), + valueAny(), }}, Bound: `cpe:/a:hp:openview_network_manager:7.51:-:~~~linux~~`, }, @@ -355,41 +359,50 @@ func TestURIUnbinding(t *testing.T) { // wfn:[part="a",vendor="foo\~bar",product="big\~money_2010",version=ANY,update=ANY,edition=ANY,language=ANY] { WFN: WFN{Attr: [NumAttr]Value{ - {Kind: ValueSet, V: "a"}, - {Kind: ValueSet, V: `foo\~bar`}, - {Kind: ValueSet, V: `big\~money_2010`}, - {Kind: ValueAny}, - {Kind: ValueAny}, - {Kind: ValueAny}, - {Kind: ValueAny}, + valueSet("a"), + valueSet(`foo\~bar`), + valueSet(`big\~money_2010`), + valueAny(), + valueAny(), + valueAny(), + valueAny(), }}, Bound: `cpe:/a:foo~bar:big%7emoney_2010`, }, // An error is raised when this URI is unbound, because it contains a special character ("%02") embedded within a valuestring. + {Bound: `cpe:/a:foo:bar:12.%02.1234`, Error: true}, + // Errors because of a disallowed character. + {Bound: `cpe:/a:redhat:openshift:4.*`, Error: true}, + // Should be fine, because it's just short. { - Bound: `cpe:/a:foo:bar:12.%02.1234`, - Error: true, + Bound: `cpe:/a:redhat:openshift:4`, + WFN: WFN{Attr: [NumAttr]Value{ + valueSet("a"), + valueSet("redhat"), + valueSet("openshift"), + valueSet("4"), + valueAny(), + valueAny(), + valueAny(), + }}, + }, + // Should be fine, because a wildcard is in an allowed state. + { + Bound: `cpe:/a:redhat:openshift:4.%02`, + WFN: WFN{Attr: [NumAttr]Value{ + valueSet("a"), + valueSet("redhat"), + valueSet("openshift"), + valueSet(`4\.*`), + valueAny(), + valueAny(), + valueAny(), + }}, }, } - for _, tc := range tt { - t.Logf("bound: %q", tc.Bound) - got, err := UnbindURI(tc.Bound) - if tc.Error { - t.Log(err) - if err == nil { - t.Error("expected error, got nil") - } - continue - } - if err != nil { - t.Error(err) - } - want := tc.WFN - t.Logf("\ngot:\t%#v\nwant:\t%#v", got, want) - if !cmp.Equal(got, want) { - t.Error(cmp.Diff(got, want)) - } - } + + t.Run("FS", inner(fsTable, UnbindFS)) + t.Run("URI", inner(uriTable, UnbindURI)) } func TestDictionary(t *testing.T) {