Ensure os.OpenFile file permissions are checked

In addition configuration file may be used to set the permission level.

Closes #53
This commit is contained in:
Grant Murphy 2016-11-12 17:57:20 -08:00
parent 423a303712
commit 1d732b8ae3
3 changed files with 40 additions and 25 deletions

View file

@ -43,7 +43,7 @@ func GetFullRuleList() map[string]RuleInfo {
// filesystem // filesystem
"G301": RuleInfo{"Poor file permissions used when creating a directory", rules.NewMkdirPerms}, "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}, "G303": RuleInfo{"Creating tempfile using a predictable path", rules.NewBadTempFile},
// crypto // crypto

View file

@ -17,52 +17,65 @@ package rules
import ( import (
"fmt" "fmt"
"go/ast" "go/ast"
"regexp" "strconv"
gas "github.com/GoASTScanner/gas/core" gas "github.com/GoASTScanner/gas/core"
) )
type FilePermissions struct { type FilePermissions struct {
gas.MetaData 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) { func (r *FilePermissions) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) {
if node := gas.MatchCall(n, r.pattern); node != nil { if callexpr, matched := gas.MatchCallByPackage(n, c, r.pkg, r.calls...); matched {
if val, err := gas.GetInt(node.Args[1]); err == nil && val > r.mode { 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 gas.NewIssue(c, n, r.What, r.Severity, r.Confidence), nil
} }
} }
return nil, nil return nil, nil
} }
func NewChmodPerms(conf map[string]interface{}) (r gas.Rule, n ast.Node) { func NewFilePerms(conf map[string]interface{}) (gas.Rule, ast.Node) {
mode := 0600 mode := getConfiguredMode(conf, "G302", 0600)
r = &FilePermissions{ return &FilePermissions{
pattern: regexp.MustCompile(`^os\.Chmod$`), mode: mode,
mode: (int64)(mode), pkg: "os",
calls: []string{"OpenFile", "Chmod"},
MetaData: gas.MetaData{ MetaData: gas.MetaData{
Severity: gas.Medium, Severity: gas.Medium,
Confidence: gas.High, 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),
}, },
} }, (*ast.CallExpr)(nil)
n = (*ast.CallExpr)(nil)
return
} }
func NewMkdirPerms(conf map[string]interface{}) (r gas.Rule, n ast.Node) { func NewMkdirPerms(conf map[string]interface{}) (gas.Rule, ast.Node) {
mode := 0700 mode := getConfiguredMode(conf, "G301", 0700)
r = &FilePermissions{ return &FilePermissions{
pattern: regexp.MustCompile(`^(os\.Mkdir|os\.MkdirAll)$`), mode: mode,
mode: (int64)(mode), pkg: "os",
calls: []string{"Mkdir", "MkdirAll"},
MetaData: gas.MetaData{ MetaData: gas.MetaData{
Severity: gas.Medium, Severity: gas.Medium,
Confidence: gas.High, Confidence: gas.High,
What: fmt.Sprintf("Expect directory permissions to be %#o or less", mode), What: fmt.Sprintf("Expect directory permissions to be %#o or less", mode),
}, },
} }, (*ast.CallExpr)(nil)
n = (*ast.CallExpr)(nil)
return
} }

View file

@ -23,7 +23,7 @@ import (
func TestChmod(t *testing.T) { func TestChmod(t *testing.T) {
config := map[string]interface{}{"ignoreNosec": false} config := map[string]interface{}{"ignoreNosec": false}
analyzer := gas.NewAnalyzer(config, nil) analyzer := gas.NewAnalyzer(config, nil)
analyzer.AddRule(NewChmodPerms(config)) analyzer.AddRule(NewFilePerms(config))
issues := gasTestRunner(` issues := gasTestRunner(`
package main package main
@ -31,9 +31,11 @@ func TestChmod(t *testing.T) {
func main() { func main() {
os.Chmod("/tmp/somefile", 0777) os.Chmod("/tmp/somefile", 0777)
os.Chmod("/tmp/someotherfile", 0600) 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) }`, analyzer)
checkTestResults(t, issues, 1, "Expect chmod permissions") checkTestResults(t, issues, 2, "Expect file permissions")
} }
func TestMkdir(t *testing.T) { func TestMkdir(t *testing.T) {