Browse Source

cmd/geth: add test to verify regexps in version check (#21962)

Martin Holst Swende 4 years ago
parent
commit
915643a3e5
2 changed files with 72 additions and 13 deletions
  1. 15 7
      cmd/geth/testdata/vcheck/vulnerabilities.json
  2. 57 6
      cmd/geth/version_check_test.go

+ 15 - 7
cmd/geth/testdata/vcheck/data2.json → cmd/geth/testdata/vcheck/vulnerabilities.json

@@ -7,28 +7,32 @@
     "links": [
       "https://github.com/ethereum/go-ethereum/pull/21793",
       "https://blog.ethereum.org/2020/11/12/geth_security_release/",
-      "https://github.com/ethereum/go-ethereum/commit/567d41d9363706b4b13ce0903804e8acf214af49"
+      "https://github.com/ethereum/go-ethereum/commit/567d41d9363706b4b13ce0903804e8acf214af49",
+      "https://github.com/ethereum/go-ethereum/security/advisories/GHSA-v592-xf75-856p"
     ],
     "introduced": "v1.6.0",
     "fixed": "v1.9.24",
     "published": "2020-11-12",
     "severity": "Medium",
-    "check": "Geth\\/v1\\.(6|7|8)\\..*|Geth\\/v1\\.9\\.2(1|2|3)-.*",
-    "CVE": "correct"
+    "CVE": "CVE-2020-26240",
+    "check": "Geth\\/v1\\.(6|7|8)\\..*|Geth\\/v1\\.9\\.\\d-.*|Geth\\/v1\\.9\\.1.*|Geth\\/v1\\.9\\.2(0|1|2|3)-.*"
   },
   {
-    "name": "GoCrash",
+    "name": "Denial of service due to Go CVE-2020-28362",
     "uid": "GETH-2020-02",
     "summary": "A denial-of-service issue can be used to crash Geth nodes during block processing, due to an underlying bug in Go (CVE-2020-28362) versions < `1.15.5`, or `<1.14.12`",
     "description": "The DoS issue can be used to crash all Geth nodes during block processing, the effects of which would be that a major part of the Ethereum network went offline.\n\nOutside of Go-Ethereum, the issue is most likely relevant for all forks of Geth (such as TurboGeth or ETC’s core-geth) which is built with versions of Go which contains the vulnerability.",
     "links": [
       "https://blog.ethereum.org/2020/11/12/geth_security_release/",
       "https://groups.google.com/g/golang-announce/c/NpBGTTmKzpM",
-      "https://github.com/golang/go/issues/42552"
+      "https://github.com/golang/go/issues/42552",
+      "https://github.com/ethereum/go-ethereum/security/advisories/GHSA-m6gx-rhvj-fh52"
     ],
+    "introduced": "v0.0.0",
     "fixed": "v1.9.24",
     "published": "2020-11-12",
     "severity": "Critical",
+    "CVE": "CVE-2020-28362",
     "check": "Geth.*\\/go1\\.(11(.*)|12(.*)|13(.*)|14|14\\.(\\d|10|11|)|15|15\\.[0-4])$"
   },
   {
@@ -37,12 +41,14 @@
     "summary": "A consensus flaw in Geth, related to `datacopy` precompile",
     "description": "Geth erroneously performed a 'shallow' copy when the precompiled `datacopy` (at `0x00...04`) was invoked. An attacker could deploy a contract that uses the shallow copy to corrupt the contents of the `RETURNDATA`, thus causing a consensus failure.",
     "links": [
-      "https://blog.ethereum.org/2020/11/12/geth_security_release/"
+      "https://blog.ethereum.org/2020/11/12/geth_security_release/",
+      "https://github.com/ethereum/go-ethereum/security/advisories/GHSA-69v6-xc2j-r2jf"
     ],
     "introduced": "v1.9.7",
     "fixed": "v1.9.17",
     "published": "2020-11-12",
     "severity": "Critical",
+    "CVE": "CVE-2020-26241",
     "check": "Geth\\/v1\\.9\\.(7|8|9|10|11|12|13|14|15|16).*$"
   },
   {
@@ -51,12 +57,14 @@
     "summary": "A denial-of-service issue can be used to crash Geth nodes during block processing",
     "description": "Full details to be disclosed at a later date",
     "links": [
-      "https://blog.ethereum.org/2020/11/12/geth_security_release/"
+      "https://blog.ethereum.org/2020/11/12/geth_security_release/",
+      "https://github.com/ethereum/go-ethereum/security/advisories/GHSA-jm5c-rv3w-w83m"
     ],
     "introduced": "v1.9.16",
     "fixed": "v1.9.18",
     "published": "2020-11-12",
     "severity": "Critical",
+    "CVE": "CVE-2020-26242",
     "check": "Geth\\/v1\\.9.(16|17).*$"
   }
 ]

+ 57 - 6
cmd/geth/version_check_test.go

@@ -18,8 +18,12 @@ package main
 
 import (
 	"encoding/json"
+	"fmt"
 	"io/ioutil"
 	"path/filepath"
+	"regexp"
+	"strconv"
+	"strings"
 	"testing"
 )
 
@@ -64,16 +68,63 @@ func testVerification(t *testing.T, pubkey, sigdir string) {
 	}
 }
 
-func TestJson(t *testing.T) {
-	data, _ := ioutil.ReadFile("./testdata/vcheck/data2.json")
+func versionUint(v string) int {
+	mustInt := func(s string) int {
+		a, err := strconv.Atoi(s)
+		if err != nil {
+			panic(v)
+		}
+		return a
+	}
+	components := strings.Split(strings.TrimPrefix(v, "v"), ".")
+	a := mustInt(components[0])
+	b := mustInt(components[1])
+	c := mustInt(components[2])
+	return a*100*100 + b*100 + c
+}
+
+// TestMatching can be used to check that the regexps are correct
+func TestMatching(t *testing.T) {
+	data, _ := ioutil.ReadFile("./testdata/vcheck/vulnerabilities.json")
 	var vulns []vulnJson
 	if err := json.Unmarshal(data, &vulns); err != nil {
 		t.Fatal(err)
 	}
-	if len(vulns) == 0 {
-		t.Fatal("expected data, got none")
+	check := func(version string) {
+		vFull := fmt.Sprintf("Geth/%v-unstable-15339cf1-20201204/linux-amd64/go1.15.4", version)
+		for _, vuln := range vulns {
+			r, err := regexp.Compile(vuln.Check)
+			vulnIntro := versionUint(vuln.Introduced)
+			vulnFixed := versionUint(vuln.Fixed)
+			current := versionUint(version)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if vuln.Name == "Denial of service due to Go CVE-2020-28362" {
+				// this one is not tied to geth-versions
+				continue
+			}
+			if vulnIntro <= current && vulnFixed > current {
+				// Should be vulnerable
+				if !r.MatchString(vFull) {
+					t.Errorf("Should be vulnerable, version %v, intro: %v, fixed: %v %v %v",
+						version, vuln.Introduced, vuln.Fixed, vuln.Name, vuln.Check)
+				}
+			} else {
+				if r.MatchString(vFull) {
+					t.Errorf("Should not be flagged vulnerable, version %v, intro: %v, fixed: %v %v %d %d %d",
+						version, vuln.Introduced, vuln.Fixed, vuln.Name, vulnIntro, current, vulnFixed)
+				}
+			}
+
+		}
 	}
-	if have, want := vulns[0].CVE, "correct"; have != want {
-		t.Errorf("have %v, want %v", have, want)
+	for major := 1; major < 2; major++ {
+		for minor := 0; minor < 30; minor++ {
+			for patch := 0; patch < 30; patch++ {
+				vShort := fmt.Sprintf("v%d.%d.%d", major, minor, patch)
+				check(vShort)
+			}
+		}
 	}
 }