diff --git a/README.md b/README.md index 772e124f89..817b59c196 100644 --- a/README.md +++ b/README.md @@ -143,6 +143,7 @@ directory you can supply `./...` as the input argument. - G108: Profiling endpoint automatically exposed on /debug/pprof - G109: Potential Integer overflow made by strconv.Atoi result conversion to int16/32 - G110: Potential DoS vulnerability via decompression bomb +- G111: Potential directory traversal - 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/issue.go b/issue.go index f0f028e10d..ecda70737e 100644 --- a/issue.go +++ b/issue.go @@ -63,6 +63,7 @@ var ruleToCWE = map[string]string{ "G108": "200", "G109": "190", "G110": "409", + "G111": "22", "G201": "89", "G202": "89", "G203": "79", diff --git a/report/formatter_test.go b/report/formatter_test.go index 94a2505214..b4b78f6064 100644 --- a/report/formatter_test.go +++ b/report/formatter_test.go @@ -276,10 +276,10 @@ var _ = Describe("Formatter", func() { }) Context("When using different report formats", func() { grules := []string{ - "G101", "G102", "G103", "G104", "G106", - "G107", "G109", "G110", "G201", "G202", "G203", "G204", - "G301", "G302", "G303", "G304", "G305", "G401", "G402", - "G403", "G404", "G501", "G502", "G503", "G504", "G505", + "G101", "G102", "G103", "G104", "G106", "G107", "G109", + "G110", "G111", "G201", "G202", "G203", "G204", "G301", + "G302", "G303", "G304", "G305", "G401", "G402", "G403", + "G404", "G501", "G502", "G503", "G504", "G505", } It("csv formatted report should contain the CWE mapping", func() { diff --git a/rules/directory-traversal.go b/rules/directory-traversal.go new file mode 100644 index 0000000000..04237faaeb --- /dev/null +++ b/rules/directory-traversal.go @@ -0,0 +1,64 @@ +package rules + +import ( + "go/ast" + "regexp" + + "github.com/securego/gosec/v2" +) + +type traversal struct { + pattern *regexp.Regexp + gosec.MetaData +} + +func (r *traversal) ID() string { + return r.MetaData.ID +} + +func (r *traversal) Match(n ast.Node, ctx *gosec.Context) (*gosec.Issue, error) { + switch node := n.(type) { + case *ast.CallExpr: + return r.matchCallExpr(node, ctx) + } + return nil, nil +} + +func (r *traversal) matchCallExpr(assign *ast.CallExpr, ctx *gosec.Context) (*gosec.Issue, error) { + for _, i := range assign.Args { + if basiclit, ok1 := i.(*ast.BasicLit); ok1 { + if fun, ok2 := assign.Fun.(*ast.SelectorExpr); ok2 { + if x, ok3 := fun.X.(*ast.Ident); ok3 { + string := x.Name + "." + fun.Sel.Name + "(" + basiclit.Value + ")" + if r.pattern.MatchString(string) { + return gosec.NewIssue(ctx, assign, r.ID(), r.What, r.Severity, r.Confidence), nil + } + } + } + } + } + return nil, nil +} + +// NewDirectoryTraversal attempts to find the use of http.Dir("/") +func NewDirectoryTraversal(id string, conf gosec.Config) (gosec.Rule, []ast.Node) { + pattern := `http\.Dir\("\/"\)|http\.Dir\('\/'\)` + if val, ok := conf["G101"]; ok { + conf := val.(map[string]interface{}) + if configPattern, ok := conf["pattern"]; ok { + if cfgPattern, ok := configPattern.(string); ok { + pattern = cfgPattern + } + } + } + + return &traversal{ + pattern: regexp.MustCompile(pattern), + MetaData: gosec.MetaData{ + ID: id, + What: "Potential directory traversal", + Confidence: gosec.Medium, + Severity: gosec.Medium, + }, + }, []ast.Node{(*ast.CallExpr)(nil)} +} diff --git a/rules/rulelist.go b/rules/rulelist.go index dc9f149164..bbe72fafbd 100644 --- a/rules/rulelist.go +++ b/rules/rulelist.go @@ -73,6 +73,7 @@ func Generate(trackSuppressions bool, filters ...RuleFilter) RuleList { {"G108", "Profiling endpoint is automatically exposed", NewPprofCheck}, {"G109", "Converting strconv.Atoi result to int32/int16", NewIntegerOverflowCheck}, {"G110", "Detect io.Copy instead of io.CopyN when decompression", NewDecompressionBombCheck}, + {"G111", "Detect http.Dir('/') as a potential risk", NewDirectoryTraversal}, // injection {"G201", "SQL query construction using format string", NewSQLStrFormat}, diff --git a/rules/rules_test.go b/rules/rules_test.go index 8cce9bc8f5..d987303574 100644 --- a/rules/rules_test.go +++ b/rules/rules_test.go @@ -90,6 +90,10 @@ var _ = Describe("gosec rules", func() { runner("G110", testutils.SampleCodeG110) }) + It("should detect potential directory traversal", func() { + runner("G111", testutils.SampleCodeG111) + }) + It("should detect sql injection via format strings", func() { runner("G201", testutils.SampleCodeG201) }) diff --git a/testutils/source.go b/testutils/source.go index a943b973e9..e6459502a3 100644 --- a/testutils/source.go +++ b/testutils/source.go @@ -982,6 +982,29 @@ func main() { }`}, 0, gosec.NewConfig()}, } + // SampleCodeG111 - potential directory traversal + SampleCodeG111 = []CodeSample{ + {[]string{` +package main + +import ( + "fmt" + "log" + "net/http" + "os" +) + +func main() { + http.Handle("/bad/", http.StripPrefix("/bad/", http.FileServer(http.Dir("/")))) + http.HandleFunc("/", HelloServer) + log.Fatal(http.ListenAndServe(":"+os.Getenv("PORT"), nil)) +} + +func HelloServer(w http.ResponseWriter, r *http.Request) { + fmt.Fprintf(w, "Hello, %s!", r.URL.Path[1:]) +}`}, 1, gosec.NewConfig()}, + } + // SampleCodeG201 - SQL injection via format string SampleCodeG201 = []CodeSample{ {[]string{`