Skip to content

Commit 9c19cb6

Browse files
authored
Add check for usage of Rat.SetString in math/big with an overflow error (#819)
* 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
1 parent fb587c1 commit 9c19cb6

File tree

9 files changed

+102
-12
lines changed

9 files changed

+102
-12
lines changed

README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,7 @@ directory you can supply `./...` as the input argument.
145145
- G110: Potential DoS vulnerability via decompression bomb
146146
- G111: Potential directory traversal
147147
- G112: Potential slowloris attack
148+
- G113: Usage of Rat.SetString in math/big with an overflow (CVE-2022-23772)
148149
- G201: SQL query construction using format string
149150
- G202: SQL query construction using string concatenation
150151
- G203: Use of unescaped data in HTML templates

call_list.go

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ func (c CallList) Add(selector, ident string) {
4747
}
4848

4949
// Contains returns true if the package and function are
50-
/// members of this call list.
50+
// members of this call list.
5151
func (c CallList) Contains(selector, ident string) bool {
5252
if idents, ok := c[selector]; ok {
5353
_, found := idents[ident]
@@ -77,17 +77,26 @@ func (c CallList) ContainsPkgCallExpr(n ast.Node, ctx *Context, stripVendor bool
7777
return nil
7878
}
7979

80-
// Use only explicit path (optionally strip vendor path prefix) to reduce conflicts
81-
path, ok := GetImportPath(selector, ctx)
82-
if !ok {
83-
return nil
80+
// Selector can have two forms:
81+
// 1. A short name if a module function is called (expr.Name).
82+
// E.g., "big" if called function from math/big.
83+
// 2. A full name if a structure function is called (TypeOf(expr)).
84+
// E.g., "math/big.Rat" if called function of Rat structure from math/big.
85+
if !strings.ContainsRune(selector, '.') {
86+
// Use only explicit path (optionally strip vendor path prefix) to reduce conflicts
87+
path, ok := GetImportPath(selector, ctx)
88+
if !ok {
89+
return nil
90+
}
91+
selector = path
8492
}
93+
8594
if stripVendor {
86-
if vendorIdx := strings.Index(path, vendorPath); vendorIdx >= 0 {
87-
path = path[vendorIdx+len(vendorPath):]
95+
if vendorIdx := strings.Index(selector, vendorPath); vendorIdx >= 0 {
96+
selector = selector[vendorIdx+len(vendorPath):]
8897
}
8998
}
90-
if !c.Contains(path, ident) {
99+
if !c.Contains(selector, ident) {
91100
return nil
92101
}
93102

helpers.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -449,3 +449,12 @@ func RootPath(root string) (string, error) {
449449
root = strings.TrimSuffix(root, "...")
450450
return filepath.Abs(root)
451451
}
452+
453+
// GoVersion returns parsed version of Go from runtime
454+
func GoVersion() (int, int, int) {
455+
versionParts := strings.Split(runtime.Version(), ".")
456+
major, _ := strconv.Atoi(versionParts[0][2:])
457+
minor, _ := strconv.Atoi(versionParts[1])
458+
build, _ := strconv.Atoi(versionParts[2])
459+
return major, minor, build
460+
}

issue.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ var ruleToCWE = map[string]string{
6565
"G110": "409",
6666
"G111": "22",
6767
"G112": "400",
68+
"G113": "190",
6869
"G201": "89",
6970
"G202": "89",
7071
"G203": "79",
@@ -182,7 +183,7 @@ func NewIssue(ctx *Context, node ast.Node, ruleID, desc string, severity Score,
182183

183184
var code string
184185
if file, err := os.Open(fobj.Name()); err == nil {
185-
defer file.Close() //#nosec
186+
defer file.Close() // #nosec
186187
s := codeSnippetStartLine(node, fobj)
187188
e := codeSnippetEndLine(node, fobj)
188189
code, err = codeSnippet(file, s, e, node)

report/formatter_test.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -277,9 +277,10 @@ var _ = Describe("Formatter", func() {
277277
Context("When using different report formats", func() {
278278
grules := []string{
279279
"G101", "G102", "G103", "G104", "G106", "G107", "G109",
280-
"G110", "G111", "G112", "G201", "G202", "G203", "G204", "G301",
281-
"G302", "G303", "G304", "G305", "G401", "G402", "G403",
282-
"G404", "G501", "G502", "G503", "G504", "G505",
280+
"G110", "G111", "G112", "G113", "G201", "G202", "G203",
281+
"G204", "G301", "G302", "G303", "G304", "G305", "G401",
282+
"G402", "G403", "G404", "G501", "G502", "G503", "G504",
283+
"G505", "G601",
283284
}
284285

285286
It("csv formatted report should contain the CWE mapping", func() {

rules/math_big_rat.go

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
package rules
2+
3+
import (
4+
"go/ast"
5+
6+
"github.com/securego/gosec/v2"
7+
)
8+
9+
type usingOldMathBig struct {
10+
gosec.MetaData
11+
calls gosec.CallList
12+
}
13+
14+
func (r *usingOldMathBig) ID() string {
15+
return r.MetaData.ID
16+
}
17+
18+
func (r *usingOldMathBig) Match(node ast.Node, ctx *gosec.Context) (gi *gosec.Issue, err error) {
19+
if callExpr := r.calls.ContainsPkgCallExpr(node, ctx, false); callExpr == nil {
20+
return nil, nil
21+
}
22+
23+
confidence := gosec.Low
24+
major, minor, build := gosec.GoVersion()
25+
if major == 1 && (minor == 16 && build < 14 || minor == 17 && build < 7) {
26+
confidence = gosec.Medium
27+
}
28+
29+
return gosec.NewIssue(ctx, node, r.ID(), r.What, r.Severity, confidence), nil
30+
}
31+
32+
// NewUsingOldMathBig rule detects the use of Rat.SetString from math/big.
33+
func NewUsingOldMathBig(id string, _ gosec.Config) (gosec.Rule, []ast.Node) {
34+
calls := gosec.NewCallList()
35+
calls.Add("math/big.Rat", "SetString")
36+
return &usingOldMathBig{
37+
calls: calls,
38+
MetaData: gosec.MetaData{
39+
ID: id,
40+
What: "Potential uncontrolled memory consumption in Rat.SetString (CVE-2022-23772)",
41+
Severity: gosec.High,
42+
},
43+
}, []ast.Node{(*ast.CallExpr)(nil)}
44+
}

rules/rulelist.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ func Generate(trackSuppressions bool, filters ...RuleFilter) RuleList {
7575
{"G110", "Detect io.Copy instead of io.CopyN when decompression", NewDecompressionBombCheck},
7676
{"G111", "Detect http.Dir('/') as a potential risk", NewDirectoryTraversal},
7777
{"G112", "Detect ReadHeaderTimeout not configured as a potential risk", NewSlowloris},
78+
{"G113", "Usage of Rat.SetString in math/big with an overflow", NewUsingOldMathBig},
7879

7980
// injection
8081
{"G201", "SQL query construction using format string", NewSQLStrFormat},

rules/rules_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,10 @@ var _ = Describe("gosec rules", func() {
9898
runner("G112", testutils.SampleCodeG112)
9999
})
100100

101+
It("should detect potential uncontrolled memory consumption in Rat.SetString", func() {
102+
runner("G113", testutils.SampleCodeG113)
103+
})
104+
101105
It("should detect sql injection via format strings", func() {
102106
runner("G201", testutils.SampleCodeG201)
103107
})

testutils/source.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1052,6 +1052,26 @@ func HelloServer(w http.ResponseWriter, r *http.Request) {
10521052
`}, 0, gosec.NewConfig()},
10531053
}
10541054

1055+
// SampleCodeG113 - Usage of Rat.SetString in math/big with an overflow
1056+
SampleCodeG113 = []CodeSample{
1057+
{[]string{
1058+
`
1059+
package main
1060+
1061+
import (
1062+
"math/big"
1063+
"fmt"
1064+
)
1065+
1066+
func main() {
1067+
r := big.Rat{}
1068+
r.SetString("13e-9223372036854775808")
1069+
1070+
fmt.Println(r)
1071+
}`,
1072+
}, 1, gosec.NewConfig()},
1073+
}
1074+
10551075
// SampleCodeG201 - SQL injection via format string
10561076
SampleCodeG201 = []CodeSample{
10571077
{[]string{`

0 commit comments

Comments
 (0)