From 1d732b8ae3240b346a7a231869f25ebe87ce8865 Mon Sep 17 00:00:00 2001 From: Grant Murphy Date: Sat, 12 Nov 2016 17:57:20 -0800 Subject: [PATCH] Ensure os.OpenFile file permissions are checked In addition configuration file may be used to set the permission level. Closes #53 --- rulelist.go | 2 +- rules/fileperms.go | 57 +++++++++++++++++++++++++---------------- rules/fileperms_test.go | 6 +++-- 3 files changed, 40 insertions(+), 25 deletions(-) diff --git a/rulelist.go b/rulelist.go index 9d39607..61ea56f 100644 --- a/rulelist.go +++ b/rulelist.go @@ -43,7 +43,7 @@ func GetFullRuleList() map[string]RuleInfo { // filesystem "G301": RuleInfo{"Poor file permissions used when creating a directory", rules.NewMkdirPerms}, - "G302": RuleInfo{"Poor file permisions used with chmod", rules.NewChmodPerms}, + "G302": RuleInfo{"Poor file permisions used when creation file or using chmod", rules.NewFilePerms}, "G303": RuleInfo{"Creating tempfile using a predictable path", rules.NewBadTempFile}, // crypto diff --git a/rules/fileperms.go b/rules/fileperms.go index 6efbc33..061876d 100644 --- a/rules/fileperms.go +++ b/rules/fileperms.go @@ -17,52 +17,65 @@ package rules import ( "fmt" "go/ast" - "regexp" + "strconv" gas "github.com/GoASTScanner/gas/core" ) type FilePermissions struct { gas.MetaData - pattern *regexp.Regexp - mode int64 + mode int64 + pkg string + calls []string +} + +func getConfiguredMode(conf map[string]interface{}, configKey string, defaultMode int64) int64 { + var mode int64 = defaultMode + if value, ok := conf[configKey]; ok { + switch value.(type) { + case int64: + mode = value.(int64) + case string: + mode, _ = strconv.ParseInt(value.(string), 0, 64) + } + } + return mode } func (r *FilePermissions) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) { - if node := gas.MatchCall(n, r.pattern); node != nil { - if val, err := gas.GetInt(node.Args[1]); err == nil && val > r.mode { + if callexpr, matched := gas.MatchCallByPackage(n, c, r.pkg, r.calls...); matched { + modeArg := callexpr.Args[len(callexpr.Args)-1] + if mode, err := gas.GetInt(modeArg); err == nil && mode > r.mode { return gas.NewIssue(c, n, r.What, r.Severity, r.Confidence), nil } } return nil, nil } -func NewChmodPerms(conf map[string]interface{}) (r gas.Rule, n ast.Node) { - mode := 0600 - r = &FilePermissions{ - pattern: regexp.MustCompile(`^os\.Chmod$`), - mode: (int64)(mode), +func NewFilePerms(conf map[string]interface{}) (gas.Rule, ast.Node) { + mode := getConfiguredMode(conf, "G302", 0600) + return &FilePermissions{ + mode: mode, + pkg: "os", + calls: []string{"OpenFile", "Chmod"}, MetaData: gas.MetaData{ Severity: gas.Medium, Confidence: gas.High, - What: fmt.Sprintf("Expect chmod permissions to be %#o or less", mode), + What: fmt.Sprintf("Expect file permissions to be %#o or less", mode), }, - } - n = (*ast.CallExpr)(nil) - return + }, (*ast.CallExpr)(nil) } -func NewMkdirPerms(conf map[string]interface{}) (r gas.Rule, n ast.Node) { - mode := 0700 - r = &FilePermissions{ - pattern: regexp.MustCompile(`^(os\.Mkdir|os\.MkdirAll)$`), - mode: (int64)(mode), +func NewMkdirPerms(conf map[string]interface{}) (gas.Rule, ast.Node) { + mode := getConfiguredMode(conf, "G301", 0700) + return &FilePermissions{ + mode: mode, + pkg: "os", + calls: []string{"Mkdir", "MkdirAll"}, MetaData: gas.MetaData{ Severity: gas.Medium, Confidence: gas.High, What: fmt.Sprintf("Expect directory permissions to be %#o or less", mode), }, - } - n = (*ast.CallExpr)(nil) - return + }, (*ast.CallExpr)(nil) } diff --git a/rules/fileperms_test.go b/rules/fileperms_test.go index 8e433b3..c333f57 100644 --- a/rules/fileperms_test.go +++ b/rules/fileperms_test.go @@ -23,7 +23,7 @@ import ( func TestChmod(t *testing.T) { config := map[string]interface{}{"ignoreNosec": false} analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewChmodPerms(config)) + analyzer.AddRule(NewFilePerms(config)) issues := gasTestRunner(` package main @@ -31,9 +31,11 @@ func TestChmod(t *testing.T) { func main() { os.Chmod("/tmp/somefile", 0777) os.Chmod("/tmp/someotherfile", 0600) + f := os.OpenFile("/tmp/thing", os.O_CREATE|os.O_WRONLY, 0666) + f := os.OpenFile("/tmp/thing", os.O_CREATE|os.O_WRONLY, 0600) }`, analyzer) - checkTestResults(t, issues, 1, "Expect chmod permissions") + checkTestResults(t, issues, 2, "Expect file permissions") } func TestMkdir(t *testing.T) {