Add check for usage of Rat.SetString in math/big with an overflow error (#819)

hackallcode · web-flow · commit 9c19cb6501e3 · 2022-06-03T00:19:51.000+02:00
* Add check for usage of Rat.SetString in math/big with an overflow error Rat.SetString in math/big in Go before 1.16.14 and 1.17.x before 1.17.7 has an overflow that can lead to Uncontrolled Memory Consumption. It is the CVE-2022-23772. * Use ContainsPkgCallExpr instead of manual parsing
diff --git a/README.md b/README.md
@@ -145,6 +145,7 @@ directory you can supply `./...` as the input argument.
 - G110: Potential DoS vulnerability via decompression bomb
 - G111: Potential directory traversal
 - G112: Potential slowloris attack
+- G113: Usage of Rat.SetString in math/big with an overflow (CVE-2022-23772)
 - G201: SQL query construction using format string
 - G202: SQL query construction using string concatenation
 - G203: Use of unescaped data in HTML templates
diff --git a/call_list.go b/call_list.go
@@ -47,7 +47,7 @@ func (c CallList) Add(selector, ident string) {
 }
 
 // Contains returns true if the package and function are
-/// members of this call list.
+// members of this call list.
 func (c CallList) Contains(selector, ident string) bool {
 	if idents, ok := c[selector]; ok {
 		_, found := idents[ident]
@@ -77,17 +77,26 @@ func (c CallList) ContainsPkgCallExpr(n ast.Node, ctx *Context, stripVendor bool
 		return nil
 	}
 
-	// Use only explicit path (optionally strip vendor path prefix) to reduce conflicts
-	path, ok := GetImportPath(selector, ctx)
-	if !ok {
-		return nil
+	// Selector can have two forms:
+	// 1. A short name if a module function is called (expr.Name).
+	// E.g., "big" if called function from math/big.
+	// 2. A full name if a structure function is called (TypeOf(expr)).
+	// E.g., "math/big.Rat" if called function of Rat structure from math/big.
+	if !strings.ContainsRune(selector, '.') {
+		// Use only explicit path (optionally strip vendor path prefix) to reduce conflicts
+		path, ok := GetImportPath(selector, ctx)
+		if !ok {
+			return nil
+		}
+		selector = path
 	}
+
 	if stripVendor {
-		if vendorIdx := strings.Index(path, vendorPath); vendorIdx >= 0 {
-			path = path[vendorIdx+len(vendorPath):]
+		if vendorIdx := strings.Index(selector, vendorPath); vendorIdx >= 0 {
+			selector = selector[vendorIdx+len(vendorPath):]
 		}
 	}
-	if !c.Contains(path, ident) {
+	if !c.Contains(selector, ident) {
 		return nil
 	}
 
diff --git a/helpers.go b/helpers.go
@@ -449,3 +449,12 @@ func RootPath(root string) (string, error) {
 	root = strings.TrimSuffix(root, "...")
 	return filepath.Abs(root)
 }
+
+// GoVersion returns parsed version of Go from runtime
+func GoVersion() (int, int, int) {
+	versionParts := strings.Split(runtime.Version(), ".")
+	major, _ := strconv.Atoi(versionParts[0][2:])
+	minor, _ := strconv.Atoi(versionParts[1])
+	build, _ := strconv.Atoi(versionParts[2])
+	return major, minor, build
+}
diff --git a/issue.go b/issue.go
@@ -65,6 +65,7 @@ var ruleToCWE = map[string]string{
 	"G110": "409",
 	"G111": "22",
 	"G112": "400",
+	"G113": "190",
 	"G201": "89",
 	"G202": "89",
 	"G203": "79",
@@ -182,7 +183,7 @@ func NewIssue(ctx *Context, node ast.Node, ruleID, desc string, severity Score,
 
 	var code string
 	if file, err := os.Open(fobj.Name()); err == nil {
-		defer file.Close() //#nosec
+		defer file.Close() // #nosec
 		s := codeSnippetStartLine(node, fobj)
 		e := codeSnippetEndLine(node, fobj)
 		code, err = codeSnippet(file, s, e, node)
diff --git a/report/formatter_test.go b/report/formatter_test.go
@@ -277,9 +277,10 @@ var _ = Describe("Formatter", func() {
 	Context("When using different report formats", func() {
 		grules := []string{
 			"G101", "G102", "G103", "G104", "G106", "G107", "G109",
-			"G110", "G111", "G112", "G201", "G202", "G203", "G204", "G301",
-			"G302", "G303", "G304", "G305", "G401", "G402", "G403",
-			"G404", "G501", "G502", "G503", "G504", "G505",
+			"G110", "G111", "G112", "G113", "G201", "G202", "G203",
+			"G204", "G301", "G302", "G303", "G304", "G305", "G401",
+			"G402", "G403", "G404", "G501", "G502", "G503", "G504",
+			"G505", "G601",
 		}
 
 		It("csv formatted report should contain the CWE mapping", func() {
diff --git a/rules/math_big_rat.go b/rules/math_big_rat.go
@@ -0,0 +1,44 @@
+package rules
+
+import (
+	"go/ast"
+
+	"github.com/securego/gosec/v2"
+)
+
+type usingOldMathBig struct {
+	gosec.MetaData
+	calls gosec.CallList
+}
+
+func (r *usingOldMathBig) ID() string {
+	return r.MetaData.ID
+}
+
+func (r *usingOldMathBig) Match(node ast.Node, ctx *gosec.Context) (gi *gosec.Issue, err error) {
+	if callExpr := r.calls.ContainsPkgCallExpr(node, ctx, false); callExpr == nil {
+		return nil, nil
+	}
+
+	confidence := gosec.Low
+	major, minor, build := gosec.GoVersion()
+	if major == 1 && (minor == 16 && build < 14 || minor == 17 && build < 7) {
+		confidence = gosec.Medium
+	}
+
+	return gosec.NewIssue(ctx, node, r.ID(), r.What, r.Severity, confidence), nil
+}
+
+// NewUsingOldMathBig rule detects the use of Rat.SetString from math/big.
+func NewUsingOldMathBig(id string, _ gosec.Config) (gosec.Rule, []ast.Node) {
+	calls := gosec.NewCallList()
+	calls.Add("math/big.Rat", "SetString")
+	return &usingOldMathBig{
+		calls: calls,
+		MetaData: gosec.MetaData{
+			ID:       id,
+			What:     "Potential uncontrolled memory consumption in Rat.SetString (CVE-2022-23772)",
+			Severity: gosec.High,
+		},
+	}, []ast.Node{(*ast.CallExpr)(nil)}
+}
diff --git a/rules/rulelist.go b/rules/rulelist.go
@@ -75,6 +75,7 @@ func Generate(trackSuppressions bool, filters ...RuleFilter) RuleList {
 		{"G110", "Detect io.Copy instead of io.CopyN when decompression", NewDecompressionBombCheck},
 		{"G111", "Detect http.Dir('/') as a potential risk", NewDirectoryTraversal},
 		{"G112", "Detect ReadHeaderTimeout not configured as a potential risk", NewSlowloris},
+		{"G113", "Usage of Rat.SetString in math/big with an overflow", NewUsingOldMathBig},
 
 		// injection
 		{"G201", "SQL query construction using format string", NewSQLStrFormat},
diff --git a/rules/rules_test.go b/rules/rules_test.go
@@ -98,6 +98,10 @@ var _ = Describe("gosec rules", func() {
 			runner("G112", testutils.SampleCodeG112)
 		})
 
+		It("should detect potential uncontrolled memory consumption in Rat.SetString", func() {
+			runner("G113", testutils.SampleCodeG113)
+		})
+
 		It("should detect sql injection via format strings", func() {
 			runner("G201", testutils.SampleCodeG201)
 		})
diff --git a/testutils/source.go b/testutils/source.go
@@ -1052,6 +1052,26 @@ func HelloServer(w http.ResponseWriter, r *http.Request) {
 		`}, 0, gosec.NewConfig()},
 	}
 
+	// SampleCodeG113 - Usage of Rat.SetString in math/big with an overflow
+	SampleCodeG113 = []CodeSample{
+		{[]string{
+			`
+package main
+
+import (
+	"math/big"
+	"fmt"
+)
+
+func main() {
+	r := big.Rat{}
+	r.SetString("13e-9223372036854775808")
+
+	fmt.Println(r)
+}`,
+		}, 1, gosec.NewConfig()},
+	}
+
 	// SampleCodeG201 - SQL injection via format string
 	SampleCodeG201 = []CodeSample{
 		{[]string{`
