Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Jun 2, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
3 changes: 2 additions & 1 deletion issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ var ruleToCWE = map[string]string{
"G110": "409",
"G111": "22",
"G112": "400",
"G113": "190",
"G201": "89",
"G202": "89",
"G203": "79",
Expand Down Expand Up @@ -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)
Expand Down
7 changes: 4 additions & 3 deletions report/formatter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
50 changes: 50 additions & 0 deletions rules/math_big_rat.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package rules

import (
"go/ast"

"github.com/securego/gosec/v2"
)

type usingOldMathBig struct {
gosec.MetaData
}

func (r *usingOldMathBig) ID() string {
return r.MetaData.ID
}

func (r *usingOldMathBig) Match(node ast.Node, ctx *gosec.Context) (gi *gosec.Issue, err error) {
callExpr, callExprOk := node.(*ast.CallExpr)
if !callExprOk {
return nil, nil
}

packageName, callName, callInfoOk := gosec.GetCallInfo(callExpr, ctx)
if callInfoOk != nil {
return nil, nil
}

if packageName != "math/big.Rat" || callName != "SetString" {
hackallcode marked this conversation as resolved.
Show resolved Hide resolved
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) {
return &usingOldMathBig{
MetaData: gosec.MetaData{
ID: id,
What: "Potential uncontrolled memory consumption in Rat.SetString (CVE-2022-23772)",
Severity: gosec.High,
},
}, []ast.Node{(*ast.CallExpr)(nil)}
}
1 change: 1 addition & 0 deletions rules/rulelist.go
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down
4 changes: 4 additions & 0 deletions rules/rules_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
Expand Down
20 changes: 20 additions & 0 deletions testutils/source.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{`
Expand Down