internal/report: improve cve5ToReport
- Consider all "affected" blocks instead of just the first one.
- More cleverly account for vendor/product/package data. For example,
ignore it if it is "n/a", or if it is merely a suffix of the module path
we already have.
- Attempt to populate version data.
Skip the test that checks if v4 and v5 are handled equivalently, as
we are now taking into account data that is only available in v5.
Change-Id: Ibf46c2ad77bad6d72b50ed21b136e5ee014a99f8
Reviewed-on: https://go-review.googlesource.com/c/vulndb/+/548057
Reviewed-by: Sarawut Wansee <sarawutwansee07@gmail.com>
Reviewed-by: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
diff --git a/internal/cveschema5/cveschema5.go b/internal/cveschema5/cveschema5.go
index 815cc5c..64a71c6 100644
--- a/internal/cveschema5/cveschema5.go
+++ b/internal/cveschema5/cveschema5.go
@@ -95,6 +95,9 @@
Fixed Version `json:"lessThan"`
Status VersionStatus `json:"status"`
VersionType string `json:"versionType"`
+
+ // Not used in Go CVEs, but supported in the schema.
+ LessThanOrEqual Version `json:"lessThanOrEqual,omitempty"`
}
type VersionStatus string
diff --git a/internal/report/cve.go b/internal/report/cve.go
index b2b4ecc..713817d 100644
--- a/internal/report/cve.go
+++ b/internal/report/cve.go
@@ -5,6 +5,7 @@
package report
import (
+ "fmt"
"regexp"
"strings"
@@ -12,6 +13,7 @@
"golang.org/x/vulndb/internal/cveschema5"
"golang.org/x/vulndb/internal/proxy"
"golang.org/x/vulndb/internal/stdlib"
+ "golang.org/x/vulndb/internal/version"
)
func vendor(modulePath string) string {
@@ -133,38 +135,9 @@
refs = append(refs, referenceFromUrl(ref.URL))
}
- // For now, use the first product name as the package path.
- // TODO(tatianabradley): Make this more sophisticated, to consider
- // all the blocks in cna.Affected, versions, etc.
- var pkgPath string
- if affected := cna.Affected; len(affected) > 0 {
- pkgPath = affected[0].Product
- }
- if stdlib.Contains(modulePath) {
- pkgPath = modulePath
- modulePath = stdlib.ModulePath
- }
- if modulePath == "" {
- modulePath = "TODO"
- }
- if pkgPath == "" {
- pkgPath = modulePath
- }
- modules := []*Module{
- {
- Module: modulePath,
- Versions: nil,
- Packages: []*Package{
- {
- Package: pkgPath,
- },
- },
- },
- }
-
r := &Report{
ID: id,
- Modules: modules,
+ Modules: affectedToModules(cna.Affected, modulePath),
Summary: Summary(cna.Title),
Description: description,
Credits: credits,
@@ -185,3 +158,129 @@
func isGoCNA5(c *cveschema5.CNAPublishedContainer) bool {
return c.ProviderMetadata.OrgID == GoOrgUUID
}
+
+func affectedToModules(as []cveschema5.Affected, modulePath string) []*Module {
+ // Use a placeholder module if there is no information on
+ // modules/packages in the CVE.
+ if len(as) == 0 {
+ return []*Module{{
+ Module: modulePath,
+ }}
+ }
+
+ var modules []*Module
+ for _, a := range as {
+ modules = append(modules, affectedToModule(&a, modulePath))
+ }
+
+ return modules
+}
+
+func affectedToModule(a *cveschema5.Affected, modulePath string) *Module {
+ var pkgPath string
+ isSet := func(s string) bool {
+ const na = "n/a"
+ return s != "" && s != na
+ }
+ switch {
+ case isSet(a.PackageName):
+ pkgPath = a.PackageName
+ case isSet(a.Product):
+ pkgPath = a.Product
+ case isSet(a.Vendor):
+ pkgPath = a.Vendor
+ default:
+ pkgPath = modulePath
+ }
+
+ // If the package path is just a suffix of the modulePath,
+ // it is probably not useful.
+ if strings.HasSuffix(modulePath, pkgPath) {
+ pkgPath = modulePath
+ }
+
+ if stdlib.Contains(pkgPath) {
+ if strings.HasPrefix(pkgPath, stdlib.ToolchainModulePath) {
+ modulePath = stdlib.ToolchainModulePath
+ } else {
+ modulePath = stdlib.ModulePath
+ }
+ }
+
+ var symbols []string
+ for _, s := range a.ProgramRoutines {
+ symbols = append(symbols, s.Name)
+ }
+
+ vs, uvs := convertVersions(a.Versions, a.DefaultStatus)
+
+ return &Module{
+ Module: modulePath,
+ Versions: vs,
+ UnsupportedVersions: uvs,
+ Packages: []*Package{
+ {
+ Package: pkgPath,
+ Symbols: symbols,
+ GOOS: a.Platforms,
+ },
+ },
+ }
+}
+
+func convertVersions(vrs []cveschema5.VersionRange, defaultStatus cveschema5.VersionStatus) (vs []VersionRange, uvs []UnsupportedVersion) {
+ for _, vr := range vrs {
+ // Version ranges starting with "n/a" don't have any meaningful data.
+ if vr.Introduced == "n/a" {
+ continue
+ }
+ v, ok := toVersionRange(&vr, defaultStatus)
+ if ok {
+ vs = append(vs, *v)
+ continue
+ }
+ uvs = append(uvs, toUnsupported(&vr, defaultStatus))
+ }
+ return vs, uvs
+}
+
+func toVersionRange(cvr *cveschema5.VersionRange, defaultStatus cveschema5.VersionStatus) (*VersionRange, bool) {
+ // For now, we only support version ranges that are easy to convert to our format.
+ if cvr.VersionType != typeSemver ||
+ cvr.LessThanOrEqual != "" ||
+ !version.IsValid(string(cvr.Introduced)) ||
+ !version.IsValid(string(cvr.Fixed)) ||
+ cvr.Status != cveschema5.StatusAffected ||
+ defaultStatus != cveschema5.StatusUnaffected {
+ return nil, false
+ }
+
+ introduced := string(cvr.Introduced)
+ if introduced == "0" {
+ introduced = ""
+ }
+
+ return &VersionRange{
+ Introduced: introduced,
+ Fixed: string(cvr.Fixed),
+ }, true
+}
+
+func toUnsupported(cvr *cveschema5.VersionRange, defaultStatus cveschema5.VersionStatus) UnsupportedVersion {
+ var version string
+ switch {
+ case cvr.Fixed != "":
+ version = fmt.Sprintf("%s from %s before %s", cvr.Status, cvr.Introduced, cvr.Fixed)
+ case cvr.LessThanOrEqual != "":
+ version = fmt.Sprintf("%s from %s to %s", cvr.Status, cvr.Introduced, cvr.Fixed)
+ default:
+ version = fmt.Sprintf("%s at %s", cvr.Status, cvr.Introduced)
+ }
+ if defaultStatus != "" {
+ version = fmt.Sprintf("%s (default: %s)", version, defaultStatus)
+ }
+ return UnsupportedVersion{
+ Version: version,
+ Type: "cve_version_range",
+ }
+}
diff --git a/internal/report/cve_test.go b/internal/report/cve_test.go
index 8e32f7a..a18ccf0 100644
--- a/internal/report/cve_test.go
+++ b/internal/report/cve_test.go
@@ -104,10 +104,12 @@
}
}
-// Check that the created report is the same for v4 and v5.
-// This is a transitional test that will be removed once we are OK
-// with divergence between v4 and v5, or if we remove support for v4 entirely.
func TestV4V5Equivalence(t *testing.T) {
+ // Skip, but leave the test in case it is needed in the course of
+ // the transition.
+ // TODO(tatianabradley): Delete this test once we have completed the
+ // transition to V5.
+ t.Skip("V4 and V5 are no longer required to be equivalent.")
if err := filepath.WalkDir(filepath.Join(testdata, "TestCVE5ToReport"), func(path string, d fs.DirEntry, err error) error {
if err != nil {
return err
diff --git a/internal/report/testdata/cve/TestCVE5ToReport/CVE-2020-9283.txtar b/internal/report/testdata/cve/TestCVE5ToReport/CVE-2020-9283.txtar
index 9a1a704..f3dc995 100644
--- a/internal/report/testdata/cve/TestCVE5ToReport/CVE-2020-9283.txtar
+++ b/internal/report/testdata/cve/TestCVE5ToReport/CVE-2020-9283.txtar
@@ -9,7 +9,7 @@
modules:
- module: golang.org/x/crypto
packages:
- - package: n/a
+ - package: golang.org/x/crypto
description: |
golang.org/x/crypto before v0.0.0-20200220183623-bac4c82f6975 for Go allows a panic during signature verification in the golang.org/x/crypto/ssh package. A client can attack an SSH server that accepts public keys. Also, a server can attack any SSH client.
cves:
diff --git a/internal/report/testdata/cve/TestCVE5ToReport/CVE-2021-3115.txtar b/internal/report/testdata/cve/TestCVE5ToReport/CVE-2021-3115.txtar
index 003879e..edf8ab4 100644
--- a/internal/report/testdata/cve/TestCVE5ToReport/CVE-2021-3115.txtar
+++ b/internal/report/testdata/cve/TestCVE5ToReport/CVE-2021-3115.txtar
@@ -7,7 +7,7 @@
-- CVE-2021-3115 --
id: PLACEHOLDER-ID
modules:
- - module: std
+ - module: cmd
packages:
- package: cmd/go
description: |
diff --git a/internal/report/testdata/cve/TestCVE5ToReport/CVE-2022-39213.txtar b/internal/report/testdata/cve/TestCVE5ToReport/CVE-2022-39213.txtar
index caf3d55..16eb31c 100644
--- a/internal/report/testdata/cve/TestCVE5ToReport/CVE-2022-39213.txtar
+++ b/internal/report/testdata/cve/TestCVE5ToReport/CVE-2022-39213.txtar
@@ -8,8 +8,11 @@
id: PLACEHOLDER-ID
modules:
- module: github.com/pandatix/go-cvss
+ unsupported_versions:
+ - version: affected at >= 0.2.0, < 0.4.0
+ type: cve_version_range
packages:
- - package: go-cvss
+ - package: github.com/pandatix/go-cvss
summary: Out-of-bounds Read in go-cvss
description: |
go-cvss is a Go module to manipulate Common Vulnerability Scoring System (CVSS). In affected versions when a full CVSS v2.0 vector string is parsed using `ParseVector`, an Out-of-Bounds Read is possible due to a lack of tests. The Go module will then panic. The problem is patched in tag `v0.4.0`, by the commit `d9d478ff0c13b8b09ace030db9262f3c2fe031f4`. Users are advised to upgrade. Users unable to upgrade may avoid this issue by parsing only CVSS v2.0 vector strings that do not have all attributes defined (e.g. `AV:N/AC:L/Au:N/C:P/I:P/A:C/E:U/RL:OF/RC:C/CDP:MH/TD:H/CR:M/IR:M/AR:M`). As stated in [SECURITY.md](https://github.com/pandatix/go-cvss/blob/master/SECURITY.md), the CPE v2.3 to refer to this Go module is `cpe:2.3:a:pandatix:go_cvss:*:*:*:*:*:*:*:*`. The entry has already been requested to the NVD CPE dictionary.
diff --git a/internal/report/testdata/cve/TestCVE5ToReport/CVE-2023-29407.txtar b/internal/report/testdata/cve/TestCVE5ToReport/CVE-2023-29407.txtar
index 76736b7..d1d2500 100644
--- a/internal/report/testdata/cve/TestCVE5ToReport/CVE-2023-29407.txtar
+++ b/internal/report/testdata/cve/TestCVE5ToReport/CVE-2023-29407.txtar
@@ -8,8 +8,14 @@
id: PLACEHOLDER-ID
modules:
- module: golang.org/x/image
+ versions:
+ - fixed: 0.10.0
packages:
- package: golang.org/x/image/tiff
+ symbols:
+ - newDecoder
+ - Decode
+ - DecodeConfig
summary: Excessive CPU consumption when decoding 0-height images in golang.org/x/image/tiff
description: |
A maliciously-crafted image can cause excessive CPU consumption in decoding. A tiled image with a height of 0 and a very large width can cause excessive CPU consumption, despite the image size (width * height) appearing to be zero.
diff --git a/internal/report/testdata/cve/TestCVE5ToReport/CVE-2023-44378.txtar b/internal/report/testdata/cve/TestCVE5ToReport/CVE-2023-44378.txtar
index 8ef1526..d3dfc27 100644
--- a/internal/report/testdata/cve/TestCVE5ToReport/CVE-2023-44378.txtar
+++ b/internal/report/testdata/cve/TestCVE5ToReport/CVE-2023-44378.txtar
@@ -8,8 +8,11 @@
id: PLACEHOLDER-ID
modules:
- module: github.com/Consensys/gnark
+ unsupported_versions:
+ - version: affected at < 0.9.0
+ type: cve_version_range
packages:
- - package: gnark
+ - package: github.com/Consensys/gnark
summary: gnark vulnerable to unsoundness in variable comparison/non-unique binary decomposition
description: |
gnark is a zk-SNARK library that offers a high-level API to design circuits. Prior to version 0.9.0, for some in-circuit values, it is possible to construct two valid decomposition to bits. In addition to the canonical decomposition of `a`, for small values there exists a second decomposition for `a+r` (where `r` is the modulus the values are being reduced by). The second decomposition was possible due to overflowing the field where the values are defined. Upgrading to version 0.9.0 should fix the issue without needing to change the calls to value comparison methods.
diff --git a/internal/report/testdata/cve/TestCVE5ToReport/CVE-2023-45141.txtar b/internal/report/testdata/cve/TestCVE5ToReport/CVE-2023-45141.txtar
index fd61569..dd39d87 100644
--- a/internal/report/testdata/cve/TestCVE5ToReport/CVE-2023-45141.txtar
+++ b/internal/report/testdata/cve/TestCVE5ToReport/CVE-2023-45141.txtar
@@ -8,8 +8,11 @@
id: PLACEHOLDER-ID
modules:
- module: github.com/gofiber/fiber
+ unsupported_versions:
+ - version: affected at < 2.50.0
+ type: cve_version_range
packages:
- - package: fiber
+ - package: github.com/gofiber/fiber
summary: CSRF Token Validation Vulnerability in fiber
description: |
Fiber is an express inspired web framework written in Go. A Cross-Site Request Forgery (CSRF) vulnerability has been identified in the application, which allows an attacker to obtain tokens and forge malicious requests on behalf of a user. This can lead to unauthorized actions being taken on the user's behalf, potentially compromising the security and integrity of the application. The vulnerability is caused by improper validation and enforcement of CSRF tokens within the application. This vulnerability has been addressed in version 2.50.0 and users are advised to upgrade. Users should take additional security measures like captchas or Two-Factor Authentication (2FA) and set Session cookies with SameSite=Lax or SameSite=Secure, and the Secure and HttpOnly attributes.
diff --git a/internal/report/testdata/cve/TestCVE5ToReport/CVE-2023-45283.txtar b/internal/report/testdata/cve/TestCVE5ToReport/CVE-2023-45283.txtar
index 091fc3d..2f7fcbe 100644
--- a/internal/report/testdata/cve/TestCVE5ToReport/CVE-2023-45283.txtar
+++ b/internal/report/testdata/cve/TestCVE5ToReport/CVE-2023-45283.txtar
@@ -8,8 +8,67 @@
id: PLACEHOLDER-ID
modules:
- module: std
+ versions:
+ - fixed: 1.20.11
+ - introduced: 1.21.0-0
+ fixed: 1.21.4
packages:
- package: path/filepath
+ goos:
+ - windows
+ symbols:
+ - Clean
+ - volumeNameLen
+ - join
+ - Abs
+ - Base
+ - Dir
+ - EvalSymlinks
+ - Glob
+ - IsLocal
+ - Join
+ - Rel
+ - Split
+ - VolumeName
+ - Walk
+ - WalkDir
+ - module: std
+ versions:
+ - fixed: 1.20.11
+ - introduced: 1.21.0-0
+ fixed: 1.21.4
+ packages:
+ - package: internal/safefilepath
+ goos:
+ - windows
+ symbols:
+ - fromFS
+ - FromFS
+ - module: std
+ versions:
+ - introduced: 1.20.11
+ fixed: 1.20.12
+ - introduced: 1.21.4
+ fixed: 1.21.5
+ packages:
+ - package: path/filepath
+ goos:
+ - windows
+ symbols:
+ - volumeNameLen
+ - Abs
+ - Base
+ - Clean
+ - Dir
+ - EvalSymlinks
+ - Glob
+ - IsLocal
+ - Join
+ - Rel
+ - Split
+ - VolumeName
+ - Walk
+ - WalkDir
summary: Insecure parsing of Windows paths with a \??\ prefix in path/filepath
description: |
The filepath package does not recognize paths with a \??\ prefix as special. On Windows, a path beginning with \??\ is a Root Local Device path equivalent to a path beginning with \\?\. Paths with a \??\ prefix may be used to access arbitrary locations on the system. For example, the path \??\c:\x is equivalent to the more common path c:\x. Before fix, Clean could convert a rooted path such as \a\..\??\b into the root local device path \??\b. Clean will now convert this to .\??\b. Similarly, Join(\, ??, b) could convert a seemingly innocent sequence of path elements into the root local device path \??\b. Join will now convert this to \.\??\b. In addition, with fix, IsAbs now correctly reports paths beginning with \??\ as absolute, and VolumeName correctly reports the \??\ prefix as a volume name. UPDATE: Go 1.20.11 and Go 1.21.4 inadvertently changed the definition of the volume name in Windows paths starting with \?, resulting in filepath.Clean(\?\c:) returning \?\c: rather than \?\c:\ (among other effects). The previous behavior has been restored.
diff --git a/internal/report/testdata/cve/TestCVE5ToReport/CVE-2023-45285.txtar b/internal/report/testdata/cve/TestCVE5ToReport/CVE-2023-45285.txtar
index def7af5..209e52d 100644
--- a/internal/report/testdata/cve/TestCVE5ToReport/CVE-2023-45285.txtar
+++ b/internal/report/testdata/cve/TestCVE5ToReport/CVE-2023-45285.txtar
@@ -7,7 +7,11 @@
-- CVE-2023-45285 --
id: PLACEHOLDER-ID
modules:
- - module: std
+ - module: cmd
+ versions:
+ - fixed: 1.20.12
+ - introduced: 1.21.0-0
+ fixed: 1.21.5
packages:
- package: cmd/go
summary: Command 'go get' may unexpectedly fallback to insecure git in cmd/go
diff --git a/internal/report/testdata/cve/TestCVE5ToReport/CVE-2023-45286.txtar b/internal/report/testdata/cve/TestCVE5ToReport/CVE-2023-45286.txtar
index daabb2d..3a445d9 100644
--- a/internal/report/testdata/cve/TestCVE5ToReport/CVE-2023-45286.txtar
+++ b/internal/report/testdata/cve/TestCVE5ToReport/CVE-2023-45286.txtar
@@ -8,8 +8,23 @@
id: PLACEHOLDER-ID
modules:
- module: github.com/go-resty/resty/v2
+ unsupported_versions:
+ - version: 'unaffected from 0 before 2.10.0 (default: affected)'
+ type: cve_version_range
packages:
- package: github.com/go-resty/resty/v2
+ symbols:
+ - handleRequestBody
+ - Backoff
+ - Request.Delete
+ - Request.Execute
+ - Request.Get
+ - Request.Head
+ - Request.Options
+ - Request.Patch
+ - Request.Post
+ - Request.Put
+ - Request.Send
summary: HTTP request body disclosure in github.com/go-resty/resty/v2
description: |
A race condition in go-resty can result in HTTP request body disclosure across requests. This condition can be triggered by calling sync.Pool.Put with the same *bytes.Buffer more than once, when request retries are enabled and a retry occurs. The call to sync.Pool.Get will then return a bytes.Buffer that hasn't had bytes.Buffer.Reset called on it. This dirty buffer will contain the HTTP request body from an unrelated request, and go-resty will append the current HTTP request body to it, sending two bodies in one request. The sync.Pool in question is defined at package level scope, so a completely unrelated server could receive the request body.
