-
-
Notifications
You must be signed in to change notification settings - Fork 620
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merge experimental / refactor #146
Changes from 1 commit
9b08174
8df48f9
cacf21f
bf78d02
50bbc53
5160048
65b18da
026fe4c
f4b705a
6943f9e
3caf7c3
9c959ca
5a11336
27b2fd9
67dc432
d4311c9
02901b9
e3b6fd9
97cde35
cfa4327
af25ac1
25d74c6
e925d3c
4c49716
d452dcb
867d300
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,12 +18,16 @@ import ( | |
"strings" | ||
) | ||
|
||
// ImportTracker is used to normalize the packages that have been imported | ||
// by a source file. It is able to differentiate between plain imports, aliased | ||
// imports and init only imports. | ||
type ImportTracker struct { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. exported type ImportTracker should have comment or be unexported |
||
Imported map[string]string | ||
Aliased map[string]string | ||
InitOnly map[string]bool | ||
} | ||
|
||
// NewImportTracker creates an empty Import tracker instance | ||
func NewImportTracker() *ImportTracker { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. exported function NewImportTracker should have comment or be unexported There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. exported function NewImportTracker should have comment or be unexported |
||
return &ImportTracker{ | ||
make(map[string]string), | ||
|
@@ -32,6 +36,7 @@ func NewImportTracker() *ImportTracker { | |
} | ||
} | ||
|
||
// TrackPackages tracks all the imports used by the supplied packages | ||
func (t *ImportTracker) TrackPackages(pkgs ...*types.Package) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. exported method ImportTracker.TrackPackages should have comment or be unexported There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. exported method ImportTracker.TrackPackages should have comment or be unexported |
||
for _, pkg := range pkgs { | ||
t.Imported[pkg.Path()] = pkg.Name() | ||
|
@@ -42,6 +47,7 @@ func (t *ImportTracker) TrackPackages(pkgs ...*types.Package) { | |
} | ||
} | ||
|
||
// TrackImport tracks imports and handles the 'unsafe' import | ||
func (t *ImportTracker) TrackImport(n ast.Node) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. exported method ImportTracker.TrackImport should have comment or be unexported There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. exported method ImportTracker.TrackImport should have comment or be unexported |
||
if imported, ok := n.(*ast.ImportSpec); ok { | ||
path := strings.Trim(imported.Path.Value, `"`) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -52,6 +52,8 @@ type reportInfo struct { | |
Stats *gas.Metrics | ||
} | ||
|
||
// CreateReport generates a report based for the supplied issues and metrics given | ||
// the specified format. The formats currently accepted are: json, csv, html and text. | ||
func CreateReport(w io.Writer, format string, issues []*gas.Issue, metrics *gas.Metrics) error { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. exported function CreateReport should have comment or be unexported There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. exported function CreateReport should have comment or be unexported |
||
data := &reportInfo{ | ||
Issues: issues, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,6 +30,7 @@ type RuleBuilder func(c Config) (Rule, []ast.Node) | |
// type of AST node it is currently visiting. | ||
type RuleSet map[reflect.Type][]Rule | ||
|
||
// NewRuleSet constructs a new RuleSet | ||
func NewRuleSet() RuleSet { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. exported function NewRuleSet should have comment or be unexported There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. exported function NewRuleSet should have comment or be unexported |
||
return make(RuleSet) | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,13 +18,17 @@ import ( | |
"github.com/GoASTScanner/gas" | ||
) | ||
|
||
// RuleDefinition contains the description of a rule and a mechanism to | ||
// create it. | ||
type RuleDefinition struct { | ||
Description string | ||
Create gas.RuleBuilder | ||
} | ||
|
||
// RuleList is a mapping of rule ID's to rule definitions | ||
type RuleList map[string]RuleDefinition | ||
|
||
// Builders returns all the create methods for a given rule list | ||
func (rl RuleList) Builders() []gas.RuleBuilder { | ||
builders := make([]gas.RuleBuilder, 0, len(rl)) | ||
for _, def := range rl { | ||
|
@@ -33,8 +37,12 @@ func (rl RuleList) Builders() []gas.RuleBuilder { | |
return builders | ||
} | ||
|
||
// RuleFilter can be used to include or exclude a rule depending on the return | ||
// value of the function | ||
type RuleFilter func(string) bool | ||
|
||
// NewRuleFilter is a closure that will include/exclude the rule ID's based on | ||
// the supplied boolean value. | ||
func NewRuleFilter(action bool, ruleIDs ...string) RuleFilter { | ||
rulelist := make(map[string]bool) | ||
for _, rule := range ruleIDs { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would validate if the ruleID is really defined in the list. If not, I will skip it. |
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,12 +20,12 @@ import ( | |
"github.com/GoASTScanner/gas" | ||
) | ||
|
||
type TemplateCheck struct { | ||
type templateCheck struct { | ||
gas.MetaData | ||
calls gas.CallList | ||
} | ||
|
||
func (t *TemplateCheck) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) { | ||
func (t *templateCheck) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) { | ||
if node := t.calls.ContainsCallExpr(n, c); node != nil { | ||
for _, arg := range node.Args { | ||
if _, ok := arg.(*ast.BasicLit); !ok { // basic lits are safe | ||
|
@@ -36,9 +36,15 @@ func (t *TemplateCheck) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) { | |
return nil, nil | ||
} | ||
|
||
// NewTemplateCheck constructs the template check rule. This rule is used to | ||
// find use of tempaltes where HTML/JS escaping is not being used | ||
func NewTemplateCheck(conf gas.Config) (gas.Rule, []ast.Node) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. exported function NewTemplateCheck should have comment or be unexported There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. exported function NewTemplateCheck should have comment or be unexported |
||
|
||
return &TemplateCheck{ | ||
calls := gas.NewCallList() | ||
calls.Add("template", "HTML") | ||
calls.Add("template", "HTMLAttr") | ||
calls.Add("template", "JS") | ||
return &templateCheck{ | ||
calls: gas.NewCallList(), | ||
MetaData: gas.MetaData{ | ||
Severity: gas.Medium, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,7 +21,7 @@ import ( | |
"github.com/GoASTScanner/gas" | ||
) | ||
|
||
type InsecureConfigTLS struct { | ||
type insecureConfigTLS struct { | ||
MinVersion int16 | ||
MaxVersion int16 | ||
requiredType string | ||
|
@@ -37,7 +37,7 @@ func stringInSlice(a string, list []string) bool { | |
return false | ||
} | ||
|
||
func (t *InsecureConfigTLS) processTLSCipherSuites(n ast.Node, c *gas.Context) *gas.Issue { | ||
func (t *insecureConfigTLS) processTLSCipherSuites(n ast.Node, c *gas.Context) *gas.Issue { | ||
tlsConfig := gas.MatchCompLit(n, c, t.requiredType) | ||
if tlsConfig == nil { | ||
return nil | ||
|
@@ -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": | ||
|
@@ -114,7 +114,7 @@ func (t *InsecureConfigTLS) processTlsConfVal(n *ast.KeyValueExpr, c *gas.Contex | |
return nil | ||
} | ||
|
||
func (t *InsecureConfigTLS) Match(n ast.Node, c *gas.Context) (gi *gas.Issue, err error) { | ||
func (t *insecureConfigTLS) Match(n ast.Node, c *gas.Context) (gi *gas.Issue, err error) { | ||
if node := gas.MatchCompLit(n, c, t.requiredType); node != nil { | ||
for _, elt := range node.Elts { | ||
if kve, ok := elt.(*ast.KeyValueExpr); ok { | ||
|
@@ -128,9 +128,9 @@ 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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. exported function NewModernTlsCheck should have comment or be unexported There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. exported function NewModernTlsCheck should have comment or be unexported |
||
// https://wiki.mozilla.org/Security/Server_Side_TLS#Modern_compatibility | ||
return &InsecureConfigTLS{ | ||
return &insecureConfigTLS{ | ||
requiredType: "tls.Config", | ||
MinVersion: 0x0303, // TLS 1.2 only | ||
MaxVersion: 0x0303, | ||
|
@@ -143,9 +143,9 @@ 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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. exported function NewIntermediateTlsCheck should have comment or be unexported There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. exported function NewIntermediateTlsCheck should have comment or be unexported |
||
// https://wiki.mozilla.org/Security/Server_Side_TLS#Intermediate_compatibility_.28default.29 | ||
return &InsecureConfigTLS{ | ||
return &insecureConfigTLS{ | ||
requiredType: "tls.Config", | ||
MinVersion: 0x0301, // TLS 1.2, 1.1, 1.0 | ||
MaxVersion: 0x0303, | ||
|
@@ -169,9 +169,9 @@ 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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. exported function NewCompatTlsCheck should have comment or be unexported There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. exported function NewCompatTlsCheck should have comment or be unexported |
||
// https://wiki.mozilla.org/Security/Server_Side_TLS#Old_compatibility_.28default.29 | ||
return &InsecureConfigTLS{ | ||
return &insecureConfigTLS{ | ||
requiredType: "tls.Config", | ||
MinVersion: 0x0301, // TLS 1.2, 1.1, 1.0 | ||
MaxVersion: 0x0303, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,21 +20,23 @@ import ( | |
"github.com/GoASTScanner/gas" | ||
) | ||
|
||
type UsingUnsafe struct { | ||
type usingUnsafe struct { | ||
gas.MetaData | ||
pkg string | ||
calls []string | ||
} | ||
|
||
func (r *UsingUnsafe) Match(n ast.Node, c *gas.Context) (gi *gas.Issue, err error) { | ||
func (r *usingUnsafe) Match(n ast.Node, c *gas.Context) (gi *gas.Issue, err error) { | ||
if _, matches := gas.MatchCallByPackage(n, c, r.pkg, r.calls...); matches { | ||
return gas.NewIssue(c, n, r.What, r.Severity, r.Confidence), nil | ||
} | ||
return nil, nil | ||
} | ||
|
||
// NewUsingUnsafe rule detects the use of the unsafe package. This is only | ||
// really useful for auditing purposes. | ||
func NewUsingUnsafe(conf gas.Config) (gas.Rule, []ast.Node) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. exported function NewUsingUnsafe should have comment or be unexported There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. exported function NewUsingUnsafe should have comment or be unexported |
||
return &UsingUnsafe{ | ||
return &usingUnsafe{ | ||
pkg: "unsafe", | ||
calls: []string{"Alignof", "Offsetof", "Sizeof", "Pointer"}, | ||
MetaData: gas.MetaData{ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,14 +20,15 @@ type buildObj struct { | |
program *loader.Program | ||
} | ||
|
||
// TestPackage is a mock package for testing purposes | ||
type TestPackage struct { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. exported type TestPackage should have comment or be unexported There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. exported type TestPackage should have comment or be unexported |
||
Path string | ||
Files map[string]string | ||
ondisk bool | ||
build *buildObj | ||
} | ||
|
||
// NewPackage will create a new and empty package. Must call Close() to cleanup | ||
// NewTestPackage will create a new and empty package. Must call Close() to cleanup | ||
// auxilary files | ||
func NewTestPackage() *TestPackage { | ||
// Files must exist in $GOPATH | ||
|
@@ -76,8 +77,8 @@ func (p *TestPackage) Build() error { | |
return err | ||
} | ||
|
||
var packageFiles []string | ||
packageConfig := loader.Config{Build: &build.Default, ParserMode: parser.ParseComments} | ||
packageFiles := make([]string, 0) | ||
for _, filename := range basePackage.GoFiles { | ||
packageFiles = append(packageFiles, path.Join(p.Path, filename)) | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
exported type ImportTracker should have comment or be unexported