diff --git a/analyzer.go b/analyzer.go index c99e4d3..23deb65 100644 --- a/analyzer.go +++ b/analyzer.go @@ -51,7 +51,7 @@ type Metrics struct { NumFound int `json:"found"` } -// The Analyzer object is the main object of GAS. It has methods traverse an AST +// Analyzer object is the main object of GAS. It has methods traverse an AST // and invoke the correct checking rules as on each node as required. type Analyzer struct { ignoreNosec bool @@ -83,6 +83,8 @@ func NewAnalyzer(conf Config, logger *log.Logger) *Analyzer { } } +// LoadRules instantiates all the rules to be used when analyzing source +// packages func (gas *Analyzer) LoadRules(ruleDefinitions ...RuleBuilder) { for _, builder := range ruleDefinitions { r, nodes := builder(gas.config) @@ -90,6 +92,7 @@ func (gas *Analyzer) LoadRules(ruleDefinitions ...RuleBuilder) { } } +// Process kicks off the analysis process for a given package func (gas *Analyzer) Process(packagePath string) error { basePackage, err := build.Default.ImportDir(packagePath, build.ImportComment) diff --git a/call_list.go b/call_list.go index 90dcc40..909ae32 100644 --- a/call_list.go +++ b/call_list.go @@ -19,23 +19,23 @@ import ( type set map[string]bool -/// CallList is used to check for usage of specific packages -/// and functions. +// CallList is used to check for usage of specific packages +// and functions. type CallList map[string]set -/// NewCallList creates a new empty CallList +// NewCallList creates a new empty CallList func NewCallList() CallList { return make(CallList) } -/// AddAll will add several calls to the call list at once +// AddAll will add several calls to the call list at once func (c CallList) AddAll(selector string, idents ...string) { for _, ident := range idents { c.Add(selector, ident) } } -/// Add a selector and call to the call list +// Add a selector and call to the call list func (c CallList) Add(selector, ident string) { if _, ok := c[selector]; !ok { c[selector] = make(set) @@ -43,7 +43,7 @@ func (c CallList) Add(selector, ident string) { c[selector][ident] = true } -/// Contains returns true if the package and function are +// Contains returns true if the package and function are /// members of this call list. func (c CallList) Contains(selector, ident string) bool { if idents, ok := c[selector]; ok { @@ -53,7 +53,7 @@ func (c CallList) Contains(selector, ident string) bool { return false } -/// ContainsCallExpr resolves the call expression name and type +// ContainsCallExpr resolves the call expression name and type /// or package and determines if it exists within the CallList func (c CallList) ContainsCallExpr(n ast.Node, ctx *Context) *ast.CallExpr { selector, ident, err := GetCallInfo(n, ctx) diff --git a/issue.go b/issue.go index f0bc1f0..1060f43 100644 --- a/issue.go +++ b/issue.go @@ -11,6 +11,7 @@ // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License. + package gas import ( @@ -25,12 +26,15 @@ import ( type Score int const ( - Low Score = iota // Low value - Medium // Medium value - High // High value + // Low severity or confidence + Low Score = iota + // Medium severity or confidence + Medium + // High severity or confidence + High ) -// An Issue is returnd by a GAS rule if it discovers an issue with the scanned code. +// Issue is returnd by a GAS rule if it discovers an issue with the scanned code. type Issue struct { Severity Score `json:"severity"` // issue severity (how problematic it is) Confidence Score `json:"confidence"` // issue confidence (how sure we are we found it) diff --git a/issue_test.go b/issue_test.go index 54aad24..c58ffb6 100644 --- a/issue_test.go +++ b/issue_test.go @@ -78,7 +78,7 @@ var _ = Describe("Issue", func() { // Use SQL rule to check binary expr cfg := gas.NewConfig() - rule, _ := rules.NewSqlStrConcat(cfg) + rule, _ := rules.NewSQLStrConcat(cfg) issue, err := rule.Match(target, ctx) Expect(err).ShouldNot(HaveOccurred()) Expect(issue).ShouldNot(BeNil()) diff --git a/output/formatter.go b/output/formatter.go index 5de2d64..39955c0 100644 --- a/output/formatter.go +++ b/output/formatter.go @@ -24,13 +24,18 @@ import ( "github.com/GoASTScanner/gas" ) -// The output format for reported issues +// ReportFormat enumrates the output format for reported issues type ReportFormat int const ( + // ReportText is the default format that writes to stdout ReportText ReportFormat = iota // Plain text format - ReportJSON // Json format - ReportCSV // CSV format + + // ReportJSON set the output format to json + ReportJSON // Json format + + // ReportCSV set the output format to csv + ReportCSV // CSV format ) var text = `Results: diff --git a/rules/big.go b/rules/big.go index dd37b5e..00c3162 100644 --- a/rules/big.go +++ b/rules/big.go @@ -20,20 +20,22 @@ import ( "github.com/GoASTScanner/gas" ) -type UsingBigExp struct { +type usingBigExp struct { gas.MetaData pkg string calls []string } -func (r *UsingBigExp) Match(n ast.Node, c *gas.Context) (gi *gas.Issue, err error) { +func (r *usingBigExp) Match(n ast.Node, c *gas.Context) (gi *gas.Issue, err error) { if _, matched := gas.MatchCallByType(n, c, r.pkg, r.calls...); matched { return gas.NewIssue(c, n, r.What, r.Severity, r.Confidence), nil } return nil, nil } + +// NewUsingBigExp detects issues with modulus == 0 for Bignum func NewUsingBigExp(conf gas.Config) (gas.Rule, []ast.Node) { - return &UsingBigExp{ + return &usingBigExp{ pkg: "*math/big.Int", calls: []string{"Exp"}, MetaData: gas.MetaData{ diff --git a/rules/bind.go b/rules/bind.go index c3f6f63..fac0568 100644 --- a/rules/bind.go +++ b/rules/bind.go @@ -22,13 +22,13 @@ import ( ) // Looks for net.Listen("0.0.0.0") or net.Listen(":8080") -type BindsToAllNetworkInterfaces struct { +type bindsToAllNetworkInterfaces struct { gas.MetaData calls gas.CallList pattern *regexp.Regexp } -func (r *BindsToAllNetworkInterfaces) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) { +func (r *bindsToAllNetworkInterfaces) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) { callExpr := r.calls.ContainsCallExpr(n, c) if callExpr == nil { return nil, nil @@ -41,11 +41,13 @@ func (r *BindsToAllNetworkInterfaces) Match(n ast.Node, c *gas.Context) (*gas.Is return nil, nil } +// NewBindsToAllNetworkInterfaces detects socket connections that are setup to +// listen on all network interfaces. func NewBindsToAllNetworkInterfaces(conf gas.Config) (gas.Rule, []ast.Node) { calls := gas.NewCallList() calls.Add("net", "Listen") calls.Add("tls", "Listen") - return &BindsToAllNetworkInterfaces{ + return &bindsToAllNetworkInterfaces{ calls: calls, pattern: regexp.MustCompile(`^(0.0.0.0|:).*$`), MetaData: gas.MetaData{ diff --git a/rules/blacklist.go b/rules/blacklist.go index 895044e..d6699ae 100644 --- a/rules/blacklist.go +++ b/rules/blacklist.go @@ -20,60 +20,57 @@ import ( "github.com/GoASTScanner/gas" ) -type BlacklistImport struct { +type blacklistedImport struct { gas.MetaData - Path string + Blacklisted map[string]string } -func (r *BlacklistImport) Match(n ast.Node, c *gas.Context) (gi *gas.Issue, err error) { +func (r *blacklistedImport) Match(n ast.Node, c *gas.Context) (gi *gas.Issue, err error) { if node, ok := n.(*ast.ImportSpec); ok { - if r.Path == node.Path.Value && node.Name.String() != "_" { - return gas.NewIssue(c, n, r.What, r.Severity, r.Confidence), nil + description, ok := r.Blacklisted[node.Path.Value] + if ok && node.Name.String() != "_" { + return gas.NewIssue(c, n, description, r.Severity, r.Confidence), nil } } return nil, nil } -func NewBlacklist_crypto_md5(conf gas.Config) (gas.Rule, []ast.Node) { - return &BlacklistImport{ +// NewBlacklistedImports reports when a blacklisted import is being used. +// Typically when a deprecated technology is being used. +func NewBlacklistedImports(conf gas.Config, blacklist map[string]string) (gas.Rule, []ast.Node) { + return &blacklistedImport{ MetaData: gas.MetaData{ - Severity: gas.High, + Severity: gas.Medium, Confidence: gas.High, - What: "Use of weak cryptographic primitive", }, - Path: `"crypto/md5"`, + Blacklisted: blacklist, }, []ast.Node{(*ast.ImportSpec)(nil)} } -func NewBlacklist_crypto_des(conf gas.Config) (gas.Rule, []ast.Node) { - return &BlacklistImport{ - MetaData: gas.MetaData{ - Severity: gas.High, - Confidence: gas.High, - What: "Use of weak cryptographic primitive", - }, - Path: `"crypto/des"`, - }, []ast.Node{(*ast.ImportSpec)(nil)} +// NewBlacklistedImportMD5 fails if MD5 is imported +func NewBlacklistedImportMD5(conf gas.Config) (gas.Rule, []ast.Node) { + return NewBlacklistedImports(conf, map[string]string{ + "crypto/md5": "Use of weak cryptographic primitive", + }) } -func NewBlacklist_crypto_rc4(conf gas.Config) (gas.Rule, []ast.Node) { - return &BlacklistImport{ - MetaData: gas.MetaData{ - Severity: gas.High, - Confidence: gas.High, - What: "Use of weak cryptographic primitive", - }, - Path: `"crypto/rc4"`, - }, []ast.Node{(*ast.ImportSpec)(nil)} +// NewBlacklistedImportDES fails if DES is imported +func NewBlacklistedImportDES(conf gas.Config) (gas.Rule, []ast.Node) { + return NewBlacklistedImports(conf, map[string]string{ + "crypto/des": "Use of weak cryptographic primitive", + }) } -func NewBlacklist_net_http_cgi(conf gas.Config) (gas.Rule, []ast.Node) { - return &BlacklistImport{ - MetaData: gas.MetaData{ - Severity: gas.High, - Confidence: gas.High, - What: "Go versions < 1.6.3 are vulnerable to Httpoxy attack: (CVE-2016-5386)", - }, - Path: `"net/http/cgi"`, - }, []ast.Node{(*ast.ImportSpec)(nil)} +// NewBlacklistedImportRC4 fails if DES is imported +func NewBlacklistedImportRC4(conf gas.Config) (gas.Rule, []ast.Node) { + return NewBlacklistedImports(conf, map[string]string{ + "crypto/rc4": "Use of weak cryptographic primitive", + }) +} + +// NewBlacklistedImportCGI fails if CGI is imported +func NewBlacklistedImportCGI(conf gas.Config) (gas.Rule, []ast.Node) { + return NewBlacklistedImports(conf, map[string]string{ + "net/http/cgi": "Go versions < 1.6.3 are vulnerable to Httpoxy attack: (CVE-2016-5386)", + }) } diff --git a/rules/errors.go b/rules/errors.go index 147b16d..cd06ed7 100644 --- a/rules/errors.go +++ b/rules/errors.go @@ -21,7 +21,7 @@ import ( "github.com/GoASTScanner/gas" ) -type NoErrorCheck struct { +type noErrorCheck struct { gas.MetaData whitelist gas.CallList } @@ -30,7 +30,7 @@ func returnsError(callExpr *ast.CallExpr, ctx *gas.Context) int { if tv := ctx.Info.TypeOf(callExpr); tv != nil { switch t := tv.(type) { case *types.Tuple: - for pos := 0; pos < t.Len(); pos += 1 { + for pos := 0; pos < t.Len(); pos++ { variable := t.At(pos) if variable != nil && variable.Type().String() == "error" { return pos @@ -45,7 +45,7 @@ func returnsError(callExpr *ast.CallExpr, ctx *gas.Context) int { return -1 } -func (r *NoErrorCheck) Match(n ast.Node, ctx *gas.Context) (*gas.Issue, error) { +func (r *noErrorCheck) Match(n ast.Node, ctx *gas.Context) (*gas.Issue, error) { switch stmt := n.(type) { case *ast.AssignStmt: for _, expr := range stmt.Rhs { @@ -70,6 +70,7 @@ func (r *NoErrorCheck) Match(n ast.Node, ctx *gas.Context) (*gas.Issue, error) { return nil, nil } +// NewNoErrorCheck detects if the returned error is unchecked func NewNoErrorCheck(conf gas.Config) (gas.Rule, []ast.Node) { // TODO(gm) Come up with sensible defaults here. Or flip it to use a @@ -86,7 +87,7 @@ func NewNoErrorCheck(conf gas.Config) (gas.Rule, []ast.Node) { } } } - return &NoErrorCheck{ + return &noErrorCheck{ MetaData: gas.MetaData{ Severity: gas.Low, Confidence: gas.High, diff --git a/rules/fileperms.go b/rules/fileperms.go index ec8295d..d69e46f 100644 --- a/rules/fileperms.go +++ b/rules/fileperms.go @@ -22,7 +22,7 @@ import ( "github.com/GoASTScanner/gas" ) -type FilePermissions struct { +type filePermissions struct { gas.MetaData mode int64 pkg string @@ -30,7 +30,7 @@ type FilePermissions struct { } func getConfiguredMode(conf map[string]interface{}, configKey string, defaultMode int64) int64 { - var mode int64 = defaultMode + var mode = defaultMode if value, ok := conf[configKey]; ok { switch value.(type) { case int64: @@ -46,7 +46,7 @@ func getConfiguredMode(conf map[string]interface{}, configKey string, defaultMod 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 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 { @@ -56,9 +56,11 @@ func (r *FilePermissions) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) return nil, nil } +// NewFilePerms creates a rule to detect file creation with a more permissive than configured +// permission mask. func NewFilePerms(conf gas.Config) (gas.Rule, []ast.Node) { mode := getConfiguredMode(conf, "G302", 0600) - return &FilePermissions{ + return &filePermissions{ mode: mode, pkg: "os", calls: []string{"OpenFile", "Chmod"}, @@ -70,9 +72,11 @@ func NewFilePerms(conf gas.Config) (gas.Rule, []ast.Node) { }, []ast.Node{(*ast.CallExpr)(nil)} } +// NewMkdirPerms creates a rule to detect directory creation with more permissive than +// configured permission mask. func NewMkdirPerms(conf gas.Config) (gas.Rule, []ast.Node) { mode := getConfiguredMode(conf, "G301", 0700) - return &FilePermissions{ + return &filePermissions{ mode: mode, pkg: "os", calls: []string{"Mkdir", "MkdirAll"}, diff --git a/rules/hardcoded_credentials.go b/rules/hardcoded_credentials.go index f4b4806..bbba8ca 100644 --- a/rules/hardcoded_credentials.go +++ b/rules/hardcoded_credentials.go @@ -24,7 +24,7 @@ import ( "github.com/nbutton23/zxcvbn-go" ) -type Credentials struct { +type credentials struct { gas.MetaData pattern *regexp.Regexp entropyThreshold float64 @@ -40,7 +40,7 @@ func truncate(s string, n int) string { return s[:n] } -func (r *Credentials) isHighEntropyString(str string) bool { +func (r *credentials) isHighEntropyString(str string) bool { s := truncate(str, r.truncate) info := zxcvbn.PasswordStrength(s, []string{}) entropyPerChar := info.Entropy / float64(len(s)) @@ -49,7 +49,7 @@ func (r *Credentials) isHighEntropyString(str string) bool { entropyPerChar >= r.perCharThreshold)) } -func (r *Credentials) Match(n ast.Node, ctx *gas.Context) (*gas.Issue, error) { +func (r *credentials) Match(n ast.Node, ctx *gas.Context) (*gas.Issue, error) { switch node := n.(type) { case *ast.AssignStmt: return r.matchAssign(node, ctx) @@ -59,7 +59,7 @@ func (r *Credentials) Match(n ast.Node, ctx *gas.Context) (*gas.Issue, error) { return nil, nil } -func (r *Credentials) matchAssign(assign *ast.AssignStmt, ctx *gas.Context) (*gas.Issue, error) { +func (r *credentials) matchAssign(assign *ast.AssignStmt, ctx *gas.Context) (*gas.Issue, error) { for _, i := range assign.Lhs { if ident, ok := i.(*ast.Ident); ok { if r.pattern.MatchString(ident.Name) { @@ -76,7 +76,7 @@ func (r *Credentials) matchAssign(assign *ast.AssignStmt, ctx *gas.Context) (*ga return nil, nil } -func (r *Credentials) matchGenDecl(decl *ast.GenDecl, ctx *gas.Context) (*gas.Issue, error) { +func (r *credentials) matchGenDecl(decl *ast.GenDecl, ctx *gas.Context) (*gas.Issue, error) { if decl.Tok != token.CONST && decl.Tok != token.VAR { return nil, nil } @@ -100,12 +100,14 @@ func (r *Credentials) matchGenDecl(decl *ast.GenDecl, ctx *gas.Context) (*gas.Is return nil, nil } +// NewHardcodedCredentials attempts to find high entropy string constants being +// assigned to variables that appear to be related to credentials. func NewHardcodedCredentials(conf gas.Config) (gas.Rule, []ast.Node) { pattern := `(?i)passwd|pass|password|pwd|secret|token` entropyThreshold := 80.0 perCharThreshold := 3.0 ignoreEntropy := false - var truncateString int = 16 + var truncateString = 16 if val, ok := conf["G101"]; ok { conf := val.(map[string]string) if configPattern, ok := conf["pattern"]; ok { @@ -133,7 +135,7 @@ func NewHardcodedCredentials(conf gas.Config) (gas.Rule, []ast.Node) { } } - return &Credentials{ + return &credentials{ pattern: regexp.MustCompile(pattern), entropyThreshold: entropyThreshold, perCharThreshold: perCharThreshold, diff --git a/rules/rand.go b/rules/rand.go index 22ab48d..ace2e02 100644 --- a/rules/rand.go +++ b/rules/rand.go @@ -20,13 +20,13 @@ import ( "github.com/GoASTScanner/gas" ) -type WeakRand struct { +type weakRand struct { gas.MetaData funcNames []string packagePath string } -func (w *WeakRand) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) { +func (w *weakRand) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) { for _, funcName := range w.funcNames { if _, matched := gas.MatchCallByPackage(n, c, w.packagePath, funcName); matched { return gas.NewIssue(c, n, w.What, w.Severity, w.Confidence), nil @@ -36,8 +36,9 @@ func (w *WeakRand) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) { return nil, nil } +// NewWeakRandCheck detects the use of random number generator that isn't cryptographically secure func NewWeakRandCheck(conf gas.Config) (gas.Rule, []ast.Node) { - return &WeakRand{ + return &weakRand{ funcNames: []string{"Read", "Int"}, packagePath: "math/rand", MetaData: gas.MetaData{ diff --git a/rules/rsa.go b/rules/rsa.go index 8dd467b..02e9d91 100644 --- a/rules/rsa.go +++ b/rules/rsa.go @@ -21,13 +21,13 @@ import ( "github.com/GoASTScanner/gas" ) -type WeakKeyStrength struct { +type weakKeyStrength struct { gas.MetaData calls gas.CallList bits int } -func (w *WeakKeyStrength) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) { +func (w *weakKeyStrength) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) { if callExpr := w.calls.ContainsCallExpr(n, c); callExpr != nil { if bits, err := gas.GetInt(callExpr.Args[1]); err == nil && bits < (int64)(w.bits) { return gas.NewIssue(c, n, w.What, w.Severity, w.Confidence), nil @@ -36,11 +36,12 @@ func (w *WeakKeyStrength) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) return nil, nil } +// NewWeakKeyStrength builds a rule that detects RSA keys < 2048 bits func NewWeakKeyStrength(conf gas.Config) (gas.Rule, []ast.Node) { calls := gas.NewCallList() calls.Add("rsa", "GenerateKey") bits := 2048 - return &WeakKeyStrength{ + return &weakKeyStrength{ calls: calls, bits: bits, MetaData: gas.MetaData{ diff --git a/rules/rulelist.go b/rules/rulelist.go index f7eb034..833b742 100644 --- a/rules/rulelist.go +++ b/rules/rulelist.go @@ -67,8 +67,8 @@ func Generate(filters ...RuleFilter) RuleList { "G105": RuleDefinition{"Audit the use of big.Exp function", NewUsingBigExp}, // injection - "G201": RuleDefinition{"SQL query construction using format string", NewSqlStrFormat}, - "G202": RuleDefinition{"SQL query construction using string concatenation", NewSqlStrConcat}, + "G201": RuleDefinition{"SQL query construction using format string", NewSQLStrFormat}, + "G202": RuleDefinition{"SQL query construction using string concatenation", NewSQLStrConcat}, "G203": RuleDefinition{"Use of unescaped data in HTML templates", NewTemplateCheck}, "G204": RuleDefinition{"Audit use of command execution", NewSubproc}, @@ -79,15 +79,15 @@ func Generate(filters ...RuleFilter) RuleList { // crypto "G401": RuleDefinition{"Detect the usage of DES, RC4, or MD5", NewUsesWeakCryptography}, - "G402": RuleDefinition{"Look for bad TLS connection settings", NewIntermediateTlsCheck}, + "G402": RuleDefinition{"Look for bad TLS connection settings", NewIntermediateTLSCheck}, "G403": RuleDefinition{"Ensure minimum RSA key length of 2048 bits", NewWeakKeyStrength}, "G404": RuleDefinition{"Insecure random number source (rand)", NewWeakRandCheck}, // blacklist - "G501": RuleDefinition{"Import blacklist: crypto/md5", NewBlacklist_crypto_md5}, - "G502": RuleDefinition{"Import blacklist: crypto/des", NewBlacklist_crypto_des}, - "G503": RuleDefinition{"Import blacklist: crypto/rc4", NewBlacklist_crypto_rc4}, - "G504": RuleDefinition{"Import blacklist: net/http/cgi", NewBlacklist_net_http_cgi}, + "G501": RuleDefinition{"Import blacklist: crypto/md5", NewBlacklistedImportMD5}, + "G502": RuleDefinition{"Import blacklist: crypto/des", NewBlacklistedImportDES}, + "G503": RuleDefinition{"Import blacklist: crypto/rc4", NewBlacklistedImportRC4}, + "G504": RuleDefinition{"Import blacklist: net/http/cgi", NewBlacklistedImportCGI}, } for rule := range rules { diff --git a/rules/sql.go b/rules/sql.go index 316fe88..602f0ab 100644 --- a/rules/sql.go +++ b/rules/sql.go @@ -21,17 +21,17 @@ import ( "github.com/GoASTScanner/gas" ) -type SqlStatement struct { +type sqlStatement struct { gas.MetaData pattern *regexp.Regexp } -type SqlStrConcat struct { - SqlStatement +type sqlStrConcat struct { + sqlStatement } // see if we can figure out what it is -func (s *SqlStrConcat) checkObject(n *ast.Ident) bool { +func (s *sqlStrConcat) checkObject(n *ast.Ident) bool { if n.Obj != nil { return n.Obj.Kind != ast.Var && n.Obj.Kind != ast.Fun } @@ -39,7 +39,7 @@ func (s *SqlStrConcat) checkObject(n *ast.Ident) bool { } // Look for "SELECT * FROM table WHERE " + " ' OR 1=1" -func (s *SqlStrConcat) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) { +func (s *sqlStrConcat) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) { if node, ok := n.(*ast.BinaryExpr); ok { if start, ok := node.X.(*ast.BasicLit); ok { if str, e := gas.GetString(start); s.pattern.MatchString(str) && e == nil { @@ -56,9 +56,10 @@ func (s *SqlStrConcat) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) { return nil, nil } -func NewSqlStrConcat(conf gas.Config) (gas.Rule, []ast.Node) { - return &SqlStrConcat{ - SqlStatement: SqlStatement{ +// NewSQLStrConcat looks for cases where we are building SQL strings via concatenation +func NewSQLStrConcat(conf gas.Config) (gas.Rule, []ast.Node) { + return &sqlStrConcat{ + sqlStatement: sqlStatement{ pattern: regexp.MustCompile(`(?)(SELECT|DELETE|INSERT|UPDATE|INTO|FROM|WHERE) `), MetaData: gas.MetaData{ Severity: gas.Medium, @@ -69,13 +70,13 @@ func NewSqlStrConcat(conf gas.Config) (gas.Rule, []ast.Node) { }, []ast.Node{(*ast.BinaryExpr)(nil)} } -type SqlStrFormat struct { - SqlStatement +type sqlStrFormat struct { + sqlStatement calls gas.CallList } // Looks for "fmt.Sprintf("SELECT * FROM foo where '%s', userInput)" -func (s *SqlStrFormat) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) { +func (s *sqlStrFormat) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) { // TODO(gm) improve confidence if database/sql is being used if node := s.calls.ContainsCallExpr(n, c); node != nil { @@ -86,10 +87,11 @@ func (s *SqlStrFormat) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) { return nil, nil } -func NewSqlStrFormat(conf gas.Config) (gas.Rule, []ast.Node) { - rule := &SqlStrFormat{ +// NewSQLStrFormat looks for cases where we're building SQL query strings using format strings +func NewSQLStrFormat(conf gas.Config) (gas.Rule, []ast.Node) { + rule := &sqlStrFormat{ calls: gas.NewCallList(), - SqlStatement: SqlStatement{ + sqlStatement: sqlStatement{ pattern: regexp.MustCompile("(?)(SELECT|DELETE|INSERT|UPDATE|INTO|FROM|WHERE) "), MetaData: gas.MetaData{ Severity: gas.Medium, diff --git a/rules/subproc.go b/rules/subproc.go index 2453acc..b6b0ea0 100644 --- a/rules/subproc.go +++ b/rules/subproc.go @@ -21,7 +21,7 @@ import ( "github.com/GoASTScanner/gas" ) -type Subprocess struct { +type subprocess struct { gas.CallList } @@ -34,7 +34,7 @@ type Subprocess struct { // is unsafe. For example: // // syscall.Exec("echo", "foobar" + tainted) -func (r *Subprocess) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) { +func (r *subprocess) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) { if node := r.ContainsCallExpr(n, c); node != nil { for _, arg := range node.Args { if ident, ok := arg.(*ast.Ident); ok { @@ -49,8 +49,9 @@ func (r *Subprocess) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) { return nil, nil } +// NewSubproc detects cases where we are forking out to an external process func NewSubproc(conf gas.Config) (gas.Rule, []ast.Node) { - rule := &Subprocess{gas.NewCallList()} + rule := &subprocess{gas.NewCallList()} rule.Add("exec", "Command") rule.Add("syscall", "Exec") return rule, []ast.Node{(*ast.CallExpr)(nil)} diff --git a/rules/tempfiles.go b/rules/tempfiles.go index 316c337..335ef0d 100644 --- a/rules/tempfiles.go +++ b/rules/tempfiles.go @@ -21,13 +21,13 @@ import ( "github.com/GoASTScanner/gas" ) -type BadTempFile struct { +type badTempFile struct { gas.MetaData calls gas.CallList args *regexp.Regexp } -func (t *BadTempFile) Match(n ast.Node, c *gas.Context) (gi *gas.Issue, err error) { +func (t *badTempFile) Match(n ast.Node, c *gas.Context) (gi *gas.Issue, err error) { if node := t.calls.ContainsCallExpr(n, c); node != nil { if arg, e := gas.GetString(node.Args[0]); t.args.MatchString(arg) && e == nil { return gas.NewIssue(c, n, t.What, t.Severity, t.Confidence), nil @@ -36,11 +36,12 @@ func (t *BadTempFile) Match(n ast.Node, c *gas.Context) (gi *gas.Issue, err erro return nil, nil } +// NewBadTempFile detects direct writes to predictable path in temporary directory func NewBadTempFile(conf gas.Config) (gas.Rule, []ast.Node) { calls := gas.NewCallList() calls.Add("ioutil", "WriteFile") calls.Add("os", "Create") - return &BadTempFile{ + return &badTempFile{ calls: calls, args: regexp.MustCompile(`^/tmp/.*$|^/var/tmp/.*$`), MetaData: gas.MetaData{ diff --git a/rules/tls.go b/rules/tls.go index 2a86052..00a13f5 100644 --- a/rules/tls.go +++ b/rules/tls.go @@ -62,7 +62,7 @@ func (t *insecureConfigTLS) processTLSCipherSuites(n ast.Node, c *gas.Context) * return nil } -func (t *insecureConfigTLS) processTlsConfVal(n *ast.KeyValueExpr, c *gas.Context) *gas.Issue { +func (t *insecureConfigTLS) processTLSConfVal(n *ast.KeyValueExpr, c *gas.Context) *gas.Issue { if ident, ok := n.Key.(*ast.Ident); ok { switch ident.Name { case "InsecureSkipVerify": @@ -118,7 +118,7 @@ func (t *insecureConfigTLS) Match(n ast.Node, c *gas.Context) (gi *gas.Issue, er if node := gas.MatchCompLit(n, c, t.requiredType); node != nil { for _, elt := range node.Elts { if kve, ok := elt.(*ast.KeyValueExpr); ok { - gi = t.processTlsConfVal(kve, c) + gi = t.processTLSConfVal(kve, c) if gi != nil { break } @@ -128,8 +128,8 @@ func (t *insecureConfigTLS) Match(n ast.Node, c *gas.Context) (gi *gas.Issue, er return } -// NewModernTlsCheck see: https://wiki.mozilla.org/Security/Server_Side_TLS#Modern_compatibility -func NewModernTlsCheck(conf gas.Config) (gas.Rule, []ast.Node) { +// NewModernTLSCheck see: https://wiki.mozilla.org/Security/Server_Side_TLS#Modern_compatibility +func NewModernTLSCheck(conf gas.Config) (gas.Rule, []ast.Node) { return &insecureConfigTLS{ requiredType: "tls.Config", MinVersion: 0x0303, // TLS 1.2 only @@ -143,8 +143,8 @@ func NewModernTlsCheck(conf gas.Config) (gas.Rule, []ast.Node) { }, []ast.Node{(*ast.CompositeLit)(nil)} } -// NewIntermediateTlsCheck see: https://wiki.mozilla.org/Security/Server_Side_TLS#Intermediate_compatibility_.28default.29 -func NewIntermediateTlsCheck(conf gas.Config) (gas.Rule, []ast.Node) { +// NewIntermediateTLSCheck see: https://wiki.mozilla.org/Security/Server_Side_TLS#Intermediate_compatibility_.28default.29 +func NewIntermediateTLSCheck(conf gas.Config) (gas.Rule, []ast.Node) { return &insecureConfigTLS{ requiredType: "tls.Config", MinVersion: 0x0301, // TLS 1.2, 1.1, 1.0 @@ -169,8 +169,8 @@ func NewIntermediateTlsCheck(conf gas.Config) (gas.Rule, []ast.Node) { }, []ast.Node{(*ast.CompositeLit)(nil)} } -// NewCompatTlsCheck see: https://wiki.mozilla.org/Security/Server_Side_TLS#Old_compatibility_.28default.29 -func NewCompatTlsCheck(conf gas.Config) (gas.Rule, []ast.Node) { +// NewCompatTLSCheck see: https://wiki.mozilla.org/Security/Server_Side_TLS#Old_compatibility_.28default.29 +func NewCompatTLSCheck(conf gas.Config) (gas.Rule, []ast.Node) { return &insecureConfigTLS{ requiredType: "tls.Config", MinVersion: 0x0301, // TLS 1.2, 1.1, 1.0 diff --git a/rules/weakcrypto.go b/rules/weakcrypto.go index d157407..d3adfdc 100644 --- a/rules/weakcrypto.go +++ b/rules/weakcrypto.go @@ -20,12 +20,12 @@ import ( "github.com/GoASTScanner/gas" ) -type UsesWeakCryptography struct { +type usesWeakCryptography struct { gas.MetaData blacklist map[string][]string } -func (r *UsesWeakCryptography) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) { +func (r *usesWeakCryptography) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) { for pkg, funcs := range r.blacklist { if _, matched := gas.MatchCallByPackage(n, c, pkg, funcs...); matched { @@ -35,13 +35,13 @@ func (r *UsesWeakCryptography) Match(n ast.Node, c *gas.Context) (*gas.Issue, er return nil, nil } -// Uses des.* md5.* or rc4.* +// NewUsesWeakCryptography detects uses of des.* md5.* or rc4.* func NewUsesWeakCryptography(conf gas.Config) (gas.Rule, []ast.Node) { calls := make(map[string][]string) calls["crypto/des"] = []string{"NewCipher", "NewTripleDESCipher"} calls["crypto/md5"] = []string{"New", "Sum"} calls["crypto/rc4"] = []string{"NewCipher"} - rule := &UsesWeakCryptography{ + rule := &usesWeakCryptography{ blacklist: calls, MetaData: gas.MetaData{ Severity: gas.Medium,