From 45f3b5f6710cdfe8f1a9f13298715a77e9f0d5e5 Mon Sep 17 00:00:00 2001 From: Tim Kelsey Date: Fri, 5 Aug 2016 12:58:27 +0100 Subject: [PATCH] Creating blacklist import rules Creating a new generic blacklist rule and removing the older specific ones. This will need configuration integration when we have some. The new test is immune to import aliasing but not shadowing --- rulelist.go | 4 +-- rules/blacklist.go | 66 ++++++++++++++++++++++++++++++++++++++++ rules/httpoxy.go | 50 ------------------------------ rules/httpoxy_test.go | 12 +++----- rules/weakcrypto.go | 34 ++------------------- rules/weakcrypto_test.go | 6 ++-- 6 files changed, 78 insertions(+), 94 deletions(-) create mode 100644 rules/blacklist.go delete mode 100644 rules/httpoxy.go diff --git a/rulelist.go b/rulelist.go index 8e4718b..3aa6f31 100644 --- a/rulelist.go +++ b/rulelist.go @@ -39,7 +39,7 @@ func newRulelist() rulelist { rs.rules = make(map[string]*ruleConfig) rs.overwritten = false rs.register("sql", rules.NewSqlStrConcat, rules.NewSqlStrFormat) - rs.register("crypto", rules.NewImportsWeakCryptography, rules.NewUsesWeakCryptography) + rs.register("crypto", rules.NewUsesWeakCryptography) rs.register("hardcoded", rules.NewHardcodedCredentials) rs.register("perms", rules.NewMkdirPerms, rules.NewChmodPerms) rs.register("tempfile", rules.NewBadTempFile) @@ -52,8 +52,8 @@ func newRulelist() rulelist { rs.register("templates", rules.NewTemplateCheck) rs.register("exec", rules.NewSubproc) rs.register("errors", rules.NewNoErrorCheck) - rs.register("httpoxy", rules.NewHttpoxyTest) rs.register("rand", rules.NewWeakRandCheck) + rs.register("blacklist_imports", rules.NewBlacklistImports) return rs } diff --git a/rules/blacklist.go b/rules/blacklist.go new file mode 100644 index 0000000..85505f5 --- /dev/null +++ b/rules/blacklist.go @@ -0,0 +1,66 @@ +// (c) Copyright 2016 Hewlett Packard Enterprise Development LP +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// 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 rules + +import ( + "go/ast" + + gas "github.com/HewlettPackard/gas/core" +) + +type BlacklistImports struct { + BlacklistSet map[string]gas.MetaData +} + +func (r *BlacklistImports) Match(n ast.Node, c *gas.Context) (gi *gas.Issue, err error) { + if node, ok := n.(*ast.ImportSpec); ok { + if data, ok := r.BlacklistSet[node.Path.Value]; ok { + return gas.NewIssue(c, n, data.What, data.Severity, data.Confidence), nil + } + } + return nil, nil +} + +func NewBlacklistImports() (r gas.Rule, n ast.Node) { + // TODO(tkelsey): make this configurable + // TODO(tkelsey): make it so each item can be selected/excluded individually + r = &BlacklistImports{ + BlacklistSet: map[string]gas.MetaData{ + `"crypto/md5"`: gas.MetaData{ + Severity: gas.High, + Confidence: gas.High, + What: "Use of weak cryptographic primitive", + }, + `"crypto/des"`: gas.MetaData{ + Severity: gas.High, + Confidence: gas.High, + What: "Use of weak cryptographic primitive", + }, + `"crypto/rc4"`: gas.MetaData{ + Severity: gas.High, + Confidence: gas.High, + What: "Use of weak cryptographic primitive", + }, + `"net/http/cgi"`: gas.MetaData{ + Severity: gas.High, + Confidence: gas.Low, + What: "Go code running under CGI is vulnerable to Httpoxy attack. (CVE-2016-5386)", + }, + }, + } + + n = (*ast.ImportSpec)(nil) + return +} diff --git a/rules/httpoxy.go b/rules/httpoxy.go deleted file mode 100644 index 1bf8da9..0000000 --- a/rules/httpoxy.go +++ /dev/null @@ -1,50 +0,0 @@ -// (c) Copyright 2016 Hewlett Packard Enterprise Development LP -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// 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 rules - -import ( - "go/ast" - "regexp" - - gas "github.com/HewlettPackard/gas/core" -) - -// Looks for "import net/http/cgi" -type HttpoxyTest struct { - gas.MetaData - pattern *regexp.Regexp -} - -func (r *HttpoxyTest) Match(n ast.Node, c *gas.Context) (gi *gas.Issue, err error) { - if node, ok := n.(*ast.ImportSpec); ok { - if r.pattern.MatchString(node.Path.Value) { - return gas.NewIssue(c, n, r.What, r.Severity, r.Confidence), nil - } - } - return -} - -func NewHttpoxyTest() (r gas.Rule, n ast.Node) { - r = &HttpoxyTest{ - MetaData: gas.MetaData{ - Severity: gas.High, - Confidence: gas.Low, - What: "Go code running under CGI is vulnerable to Httpoxy attack. (CVE-2016-5386)", - }, - pattern: regexp.MustCompile(`^"net/http/cgi"$`), - } - n = (*ast.ImportSpec)(nil) - return -} diff --git a/rules/httpoxy_test.go b/rules/httpoxy_test.go index 0d42260..400f7b7 100644 --- a/rules/httpoxy_test.go +++ b/rules/httpoxy_test.go @@ -22,16 +22,14 @@ import ( func TestHttpoxy(t *testing.T) { analyzer := gas.NewAnalyzer(false, nil, nil) - analyzer.AddRule(NewHttpoxyTest()) + analyzer.AddRule(NewBlacklistImports()) issues := gasTestRunner(` package main - import ( - "log" - "net/http/cgi" - ) - func main() { - }`, analyzer) + import ( + "net/http/cgi" + ) + func main() {}`, analyzer) checkTestResults(t, issues, 1, "Go code running under CGI is vulnerable to Httpoxy attack.") } diff --git a/rules/weakcrypto.go b/rules/weakcrypto.go index c5db192..7115195 100644 --- a/rules/weakcrypto.go +++ b/rules/weakcrypto.go @@ -15,42 +15,12 @@ package rules import ( - gas "github.com/HewlettPackard/gas/core" "go/ast" - "reflect" "regexp" + + gas "github.com/HewlettPackard/gas/core" ) -type ImportsWeakCryptography struct { - gas.MetaData - pattern *regexp.Regexp -} - -func (r *ImportsWeakCryptography) Match(n ast.Node, c *gas.Context) (gi *gas.Issue, err error) { - a := reflect.TypeOf(&ast.ImportSpec{}) - b := reflect.TypeOf(&ast.BasicLit{}) - if node := gas.SimpleSelect(n, a, b); node != nil { - if str, _ := gas.GetString(node); r.pattern.MatchString(str) { - return gas.NewIssue(c, n, r.What, r.Severity, r.Confidence), nil - } - } - return -} - -// Imports crypto/md5, crypto/des crypto/rc4 -func NewImportsWeakCryptography() (r gas.Rule, n ast.Node) { - r = &ImportsWeakCryptography{ - pattern: regexp.MustCompile(`crypto/md5|crypto/des|crypto/rc4`), - MetaData: gas.MetaData{ - Severity: gas.Medium, - Confidence: gas.High, - What: "Import of weak cryptographic primitive", - }, - } - n = (*ast.ImportSpec)(nil) - return -} - type UsesWeakCryptography struct { gas.MetaData pattern *regexp.Regexp diff --git a/rules/weakcrypto_test.go b/rules/weakcrypto_test.go index 388aebd..390f0ec 100644 --- a/rules/weakcrypto_test.go +++ b/rules/weakcrypto_test.go @@ -22,7 +22,7 @@ import ( func TestMD5(t *testing.T) { analyzer := gas.NewAnalyzer(false, nil, nil) - analyzer.AddRule(NewImportsWeakCryptography()) + analyzer.AddRule(NewBlacklistImports()) analyzer.AddRule(NewUsesWeakCryptography()) issues := gasTestRunner(` @@ -43,7 +43,7 @@ func TestMD5(t *testing.T) { func TestDES(t *testing.T) { analyzer := gas.NewAnalyzer(false, nil, nil) - analyzer.AddRule(NewImportsWeakCryptography()) + analyzer.AddRule(NewBlacklistImports()) analyzer.AddRule(NewUsesWeakCryptography()) issues := gasTestRunner(` @@ -82,7 +82,7 @@ func TestDES(t *testing.T) { func TestRC4(t *testing.T) { analyzer := gas.NewAnalyzer(false, nil, nil) - analyzer.AddRule(NewImportsWeakCryptography()) + analyzer.AddRule(NewBlacklistImports()) analyzer.AddRule(NewUsesWeakCryptography()) issues := gasTestRunner(`