Skip to content
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

Merged
merged 26 commits into from
Jan 5, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
9b08174
Process via packages instead of files
gcmurphy Apr 25, 2017
8df48f9
Fix to reporting to use output formats
gcmurphy Apr 26, 2017
cacf21f
Restructure to focus on lib rather than cli
gcmurphy Apr 26, 2017
bf78d02
Restructure and introduce a standalone config
gcmurphy Apr 28, 2017
50bbc53
Isolate import tracking functionality
gcmurphy May 10, 2017
5160048
Move rule definitions into own file
gcmurphy May 10, 2017
65b18da
Hack to address circular dependency in rulelist
gcmurphy May 10, 2017
026fe4c
Simplify analyzer and command line interface
gcmurphy May 10, 2017
f4b705a
Use glide to manage vendored dependencies
gcmurphy May 10, 2017
6943f9e
Major rework of codebase
gcmurphy Jul 19, 2017
3caf7c3
Add test cases
gcmurphy Sep 16, 2017
9c959ca
Issue.Line is already a string
lanzafame Oct 1, 2017
5a11336
remove commited binary
lanzafame Oct 1, 2017
27b2fd9
Merge pull request #136 from lanzafame/experimental
gcmurphy Oct 4, 2017
67dc432
use godep instead of glide
gcmurphy Dec 13, 2017
d4311c9
make it clear that these tests have not been implemented yet
gcmurphy Dec 13, 2017
02901b9
actually skip tests until implementation exists
gcmurphy Dec 13, 2017
e3b6fd9
update readme to provide info regarding package level scans
gcmurphy Dec 13, 2017
97cde35
update travis-ci to use ginkgo tests
gcmurphy Dec 13, 2017
cfa4327
fix hound-ci errors
gcmurphy Dec 13, 2017
af25ac1
fix golint errors picked up by hound-ci
gcmurphy Dec 13, 2017
25d74c6
address review comments
gcmurphy Dec 14, 2017
e925d3c
Migrated old test cases.
gcmurphy Dec 28, 2017
4c49716
move utils to separate executable
gcmurphy Dec 28, 2017
d452dcb
Fix ginko invocation
gcmurphy Jan 5, 2018
867d300
Fix lint issues
gcmurphy Jan 5, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix hound-ci errors
  • Loading branch information
gcmurphy committed Dec 13, 2017
commit cfa432729c8dbb9efa656d09ddcc511b0762ba02
2 changes: 1 addition & 1 deletion cmd/gas/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ func loadConfig(configFile string) (gas.Config, error) {
}

func loadRules(include, exclude string) rules.RuleList {
filters := make([]rules.RuleFilter, 0)
var filters []rules.RuleFilter
if include != "" {
including := strings.Split(include, ",")
filters = append(filters, rules.NewRuleFilter(false, including...))
Expand Down
6 changes: 6 additions & 0 deletions import_tracker.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

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

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

Imported map[string]string
Aliased map[string]string
InitOnly map[string]bool
}

// NewImportTracker creates an empty Import tracker instance
func NewImportTracker() *ImportTracker {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exported function NewImportTracker should have comment or be unexported

Choose a reason for hiding this comment

The 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),
Expand All @@ -32,6 +36,7 @@ func NewImportTracker() *ImportTracker {
}
}

// TrackPackages tracks all the imports used by the supplied packages
func (t *ImportTracker) TrackPackages(pkgs ...*types.Package) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exported method ImportTracker.TrackPackages should have comment or be unexported

Choose a reason for hiding this comment

The 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()
Expand All @@ -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) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exported method ImportTracker.TrackImport should have comment or be unexported

Choose a reason for hiding this comment

The 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, `"`)
Expand Down
2 changes: 2 additions & 0 deletions output/formatter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exported function CreateReport should have comment or be unexported

Choose a reason for hiding this comment

The 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,
Expand Down
1 change: 1 addition & 0 deletions rule.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exported function NewRuleSet should have comment or be unexported

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exported function NewRuleSet should have comment or be unexported

return make(RuleSet)
}
Expand Down
8 changes: 8 additions & 0 deletions rules/rulelist.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Copy link
Member

Choose a reason for hiding this comment

The 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.

Expand Down
12 changes: 9 additions & 3 deletions rules/templates.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exported function NewTemplateCheck should have comment or be unexported

Choose a reason for hiding this comment

The 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,
Expand Down
20 changes: 10 additions & 10 deletions rules/tls.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import (
"github.com/GoASTScanner/gas"
)

type InsecureConfigTLS struct {
type insecureConfigTLS struct {
MinVersion int16
MaxVersion int16
requiredType string
Expand All @@ -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
Expand All @@ -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":
Expand Down Expand Up @@ -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 {
Expand All @@ -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) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exported function NewModernTlsCheck should have comment or be unexported
func NewModernTlsCheck should be NewModernTLSCheck

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exported function NewModernTlsCheck should have comment or be unexported
func NewModernTlsCheck should be NewModernTLSCheck

// 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,
Expand All @@ -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) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exported function NewIntermediateTlsCheck should have comment or be unexported
func NewIntermediateTlsCheck should be NewIntermediateTLSCheck

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exported function NewIntermediateTlsCheck should have comment or be unexported
func NewIntermediateTlsCheck should be NewIntermediateTLSCheck

// 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,
Expand All @@ -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) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exported function NewCompatTlsCheck should have comment or be unexported
func NewCompatTlsCheck should be NewCompatTLSCheck

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exported function NewCompatTlsCheck should have comment or be unexported
func NewCompatTlsCheck should be NewCompatTLSCheck

// 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,
Expand Down
8 changes: 5 additions & 3 deletions rules/unsafe.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exported function NewUsingUnsafe should have comment or be unexported

Choose a reason for hiding this comment

The 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{
Expand Down
5 changes: 3 additions & 2 deletions testutils/pkg.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,15 @@ type buildObj struct {
program *loader.Program
}

// TestPackage is a mock package for testing purposes
type TestPackage struct {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exported type TestPackage should have comment or be unexported

Choose a reason for hiding this comment

The 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
Expand Down Expand Up @@ -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))
}
Expand Down