securego/gosec

Go security checker

gosec's Issues

#nosec comment not working

Adding a #nosec comment doesn't work. The code below is still reported as unsafe:

Code:

cmd := exec.Command("sh", "-c", config.Command) // #nosec

And Log:

Subprocess launching with variable. (Confidence: HIGH, Severity: HIGH)
  > exec.Command("sh", "-c", config.Command)

Improve test coverage

Currently unit test coverage is decent for existing rules but could be better across other modules.

$ gas git:(experimental) go test -cover ./...
? github.com/GoAstScanner/gas [no test files]
? github.com/GoAstScanner/gas/core [no test files]
? github.com/GoAstScanner/gas/output [no test files]
ok github.com/GoAstScanner/gas/rules 0.064s coverage: 90.6% of statements

TLS rule presents the same result multiple times.

Because by default all rules are run, and as there are three variants of the TLS rule whenever we get a match for cipher suites we get the same error message multiple times with little distinction between them.

For example given this test file:

package main

import (
    "crypto/tls"
    "fmt"
    "net/http"
)

func main() {
    tr := &http.Transport{
        TLSClientConfig: &tls.Config{MinVersion: 0},
    }
    client := &http.Client{Transport: tr}
    _, err := client.Get("https://golang.org/")
    if err != nil {
        fmt.Println(err)
    }
}

Executing gas gives the same error three times:

$ ../gas  ./min_version.go
Results:

[./min_version.go:11] - TLS MinVersion too low. (Confidence: HIGH, Severity: HIGH)
  > MinVersion: 0


[./min_version.go:11] - TLS MinVersion too low. (Confidence: HIGH, Severity: HIGH)
  > MinVersion: 0


[./min_version.go:11] - TLS MinVersion too low. (Confidence: HIGH, Severity: HIGH)
  > MinVersion: 0


Summary:
   Files: 1
   Lines: 18
   Nosec: 0
  Issues: 3

I think we need to perhaps revisit how we are registering rules and selecting defaults. Perhaps we need something like profiles?

Flagging one unhandled err but not another

gas is flagging this

defer func() {
	_ = file.Close()
}()

Errors unhandled. (Confidence: HIGH, Severity: LOW)
  > _ = file.Close()

but not this

defer func() {
	_ = req.Body.Close()
}()

Skip no longer accepts partial paths

Formerly, -skip=baz/*/foo.go would skip any file that ended with that pattern. More specifically, an absolute path of the form /home/travis/baz/bar/foo.go would be skipped. However, after switching from path/filepath to GitHub.com/ryanuber/go-glob to resolve globs, the same file will be scanned. This change was made in commit 1a481fa.

Subprocess checks should apply to syscall.Exec() as well

The checks for calling a subprocess look for exec.Command. It would probably make sense to apply the same checks to syscall.Exec.

Use JSON encoding module rather than templates

To make sure the json output is valid we should use the https://golang.org/pkg/encoding/json/ module instead of templates.

Warn on cgi usage

Go has a CGI package, and in some versions could be vulnerable to https://httpoxy.org/.

We could potentially warn about this..

what does 'Subprocess launching with variable.,HIGH,HIGH (gas)' mean ?

for sample code

echoCmd:="echo 123"
_, err := exec.Command("sh", "-c", echoCmd).Output()

why launching subprocess with variable is considered as an error / warning / code smell ?

Hardcoded credentials rule will flag any parser code (like for "token")

See: https://github.com/GoASTScanner/gas/blob/master/rules/hardcoded_credentials.go#L77

This is flagged as bad:

const (
	ATNStateInvalidType    = 0
	ATNStateBasic          = 1
	ATNStateRuleStart      = 2
	ATNStateBlockStart     = 3
	ATNStatePlusBlockStart = 4
	ATNStateStarBlockStart = 5
	ATNStateTokenStart     = 6
	// ...

	ATNStateInvalidStateNumber = -1
)

This would also be flagged as bad:

const (
	UsernameFieldLabel = "Username"
	// ...
)

Rule G302 checks poor file permissions on chmod but not on os.OpenFile

e.g. file, err = os.OpenFile(path, os.O_CREATE|os.O_WRONLY, 0666).

Checkstyle output format

Hi,
I'm planning to incorporate gas in the static analysis tool set I execute in the CI chain of my projects. It could be nice if gas can generate reports in Checkstyle (XML) format.
Did you have plans to generate Checkstyle-compatible results?
If you think it will nice to have checkstyle-format outpus, I can implement that micro-feature.

Regards,

"Potential hardcoded credentials" with non-string

I get this error from https://github.com/weaveworks/mesh/blob/master/router.go#L22

#109 says it should check only strings, but there are no strings there.

$ gas router.go 
[gas] 2017/01/17 15:25:18 scanning "/home/vagrant/src/github.com/weaveworks/mesh/router.go"
Results:

[/home/vagrant/src/github.com/weaveworks/mesh/router.go:22] - Potential hardcoded credentials (Confidence: LOW, Severity: HIGH)
  > const (
	tcpHeartbeat     = 30 * time.Second
	gossipInterval   = 30 * time.Second
	maxDuration      = time.Duration(math.MaxInt64)
	acceptMaxTokens  = 100
	acceptTokenDelay = 100 * time.Millisecond // [2]
)

Quiet/error-only output mode

Hi there 👋 This project really cool, and it would be convenient if gas had a CLI mode that would only output errors, or even just the error code. Basically, a way to mute this:

$ gas file.go
0
Results:

Summary:
   Files: 1
   Lines: 100
   Nosec: 0
  Issues: 0
$

Perhaps something like this, which implicitly returns an error code if there were gas Issues:

$ gas --quiet files.go
$

Or some -error-only flag that would only output if there was an error:

$ gas --error-only good_file.go
$ gas --error-only bad_file.go
[bad_file.go:100] - RSA keys should be at least 2048 bits (Confidence: HIGH, Severity: MEDIUM)
  > rsa.GenerateKey(rand.Reader, 512)

I'd love to hook gas up to a linter check and CI, but only want actionable error feedback to make it to stdout logs.

I'm also interested in #50 because I'm constructing a find statement to feed files to gas to skip both vendor and *_test.go type files (which is how I ran into this feature request):

find . -type f -name "*.go" -not -path "./vendor/*" -not -name "*_test.go" -exec gas {} \;

Add version information in builds

In preparation to move towards Beta and first official release need to embed version information with each build and move towards proper release management using milestones.

Gas checks _test files when multiple packages are found

src/pkg/violation_test.go has security violations

cd src/pkg; gas .  // no errors are reported

cd src; gas ./... // gas reports errors in violation_test.go files

cd src; gas -skip=*_test.go ./... // gas repots errors in violation_test.go

Flag potential sensitive information being logged

log.Print* could be used to log sensitive information and this practice should be avoided. Any cases where it appears that PII type information is being logged or credentials etc it should be flagged for audit purposes.

Hardcoded credentials panic

Running on a large repo, getting this panic from the hardcoded credentials check.

This is from revision 9301684.

panic: runtime error: index out of range

goroutine 1 [running]:
panic(0x2b9ba0, 0xc420012240)
	/usr/local/go/src/runtime/panic.go:500 +0x1a1
github.com/GoASTScanner/gas/rules.(*Credentials).matchGenDecl(0xc4201827e0, 0xc4206a7580, 0xc42008e1c0, 0xa40000c4206f2f80, 0x4, 0xc42008cb00)
	/Users/cs/src/go/src/github.com/GoASTScanner/gas/rules/hardcoded_credentials.go:62 +0x23e
github.com/GoASTScanner/gas/rules.(*Credentials).Match(0xc4201827e0, 0x459ea0, 0xc4206a7580, 0xc42008e1c0, 0x307101, 0x5, 0xffffffffffffffff)
	/Users/cs/src/go/src/github.com/GoASTScanner/gas/rules/hardcoded_credentials.go:34 +0x118
github.com/GoASTScanner/gas/core.(*Analyzer).Visit(0xc42008e1b0, 0x459ea0, 0xc4206a7580, 0x4587e0, 0xc42008e1b0)
	/Users/cs/src/go/src/github.com/GoASTScanner/gas/core/analyzer.go:214 +0x277
go/ast.Walk(0x4587e0, 0xc42008e1b0, 0x459ea0, 0xc4206a7580)
	/usr/local/go/src/go/ast/walk.go:52 +0x63
go/ast.Walk(0x4587e0, 0xc42008e1b0, 0x5712e0, 0xc4207ce160)
	/usr/local/go/src/go/ast/walk.go:186 +0x32bf
go/ast.walkStmtList(0x4587e0, 0xc42008e1b0, 0xc42079f200, 0x14, 0x20)
	/usr/local/go/src/go/ast/walk.go:32 +0x89
go/ast.Walk(0x4587e0, 0xc42008e1b0, 0x459c20, 0xc420574c30)
	/usr/local/go/src/go/ast/walk.go:224 +0x1d48
go/ast.Walk(0x4587e0, 0xc42008e1b0, 0x56dcc0, 0xc420574c60)
	/usr/local/go/src/go/ast/walk.go:344 +0xf03
go/ast.walkDeclList(0x4587e0, 0xc42008e1b0, 0xc4201f3080, 0x2, 0x2)
	/usr/local/go/src/go/ast/walk.go:38 +0x89
go/ast.Walk(0x4587e0, 0xc42008e1b0, 0x459e20, 0xc420322d00)
	/usr/local/go/src/go/ast/walk.go:353 +0x29e4
github.com/GoASTScanner/gas/core.(*Analyzer).process(0xc42008e1b0, 0xc42086a550, 0x42, 0x0, 0x0, 0x2800000000000001, 0x8)
	/Users/cs/src/go/src/github.com/GoASTScanner/gas/core/analyzer.go:137 +0x5b7
github.com/GoASTScanner/gas/core.(*Analyzer).Process(0xc42008e1b0, 0xc42086a550, 0x42, 0x3, 0x495a01)
	/Users/cs/src/go/src/github.com/GoASTScanner/gas/core/analyzer.go:159 +0x55
main.main.func1(0xc42086a550, 0x42, 0x45d5a0, 0xc4202865b0, 0x0, 0x0, 0x0, 0x0)
	/Users/cs/src/go/src/github.com/GoASTScanner/gas/main.go:198 +0x15e
path/filepath.walk(0xc42086a550, 0x42, 0x45d5a0, 0xc4202865b0, 0xc42019c000, 0x0, 0x0)
	/usr/local/go/src/path/filepath/path.go:351 +0x81
path/filepath.walk(0xc420412a50, 0x2e, 0x45d5a0, 0xc4202864e0, 0xc42019c000, 0x0, 0x0)
	/usr/local/go/src/path/filepath/path.go:376 +0x344
path/filepath.walk(0xc4203f08d0, 0x29, 0x45d5a0, 0xc4206c05b0, 0xc42019c000, 0x0, 0x0)
	/usr/local/go/src/path/filepath/path.go:376 +0x344
path/filepath.walk(0xc420014004, 0x1e, 0x45d5a0, 0xc42017d5f0, 0xc42019c000, 0x0, 0x1)
	/usr/local/go/src/path/filepath/path.go:376 +0x344
path/filepath.Walk(0xc420014004, 0x1e, 0xc42019c000, 0x0, 0xc420015e01)
	/usr/local/go/src/path/filepath/path.go:398 +0xd5
main.main()
	/Users/cs/src/go/src/github.com/GoASTScanner/gas/main.go:204 +0x12a5

This is on a big repo, so I don't know exactly what's killing it yet, but I'll try to narrow it down and see if I can come up with a minimal example.

G104 whitelisting

There are some functions that do return an error but are guaranteed to always return nil - so checking for errors is somewhere between pointless and a sign of risky program logic.

It'd be nice to be able to whitelist functions rather than having to annotate, e.g., every use of bytes.Buffer.Write() individually.

Improper escaping in JSON output

Running gas on the following file produces invalid JSON output:

package main

func main() {
    password := "repro for escaping bug < > & ;"
}

Produces:

$ gas -fmt json main.go
{
        "metrics": {
            "files": 1,
            "lines": 5,
            "nosec": 0,
            "issues": 1
        },
        "issues": [
        {
          "file": "main.go",
          "line": "4",
          "details": "Potential hardcoded credentials",
          "confidence": "LOW",
          "severity": "HIGH",
          "code": "password := \"repro for escaping bug \x3C \x3E & ;\""
        }
        ]
}

Note that < and > are escaped in a way that's not valid JSON.

$ gas -fmt json main.go | jq .
parse error: Invalid escape at line 15, column 72

False positives for G404

Some time in the past week we started getting false positives for G404:

Use of weak random number generator (math/rand instead of crypto/rand) (Confidence: MEDIUM, Severity: HIGH)

I haven't bisected, but e0db3f4 looks suspect

Detect panics within code base

Could be useful when auditing a code base to know about cases which call into panic without a recover clause.

#nosec doesn't work as documented

The documentation describes that any lines annotated with #nosec should be ignored, but the actual code just looks for nosec. This is causing some lines to be ignored as a false negative.

The code in core/analyzer.go looks like it just looks for 'nosec':

// ignore a node (and sub-tree) if it is tagged with a "nosec" comment
func (gas *Analyzer) ignore(n ast.Node) bool {
        if groups, ok := gas.context.Comments[n]; ok && !gas.ignoreNosec {
                for _, group := range groups {
                        if strings.Contains(group.Text(), "nosec") {
                                gas.Stats.NumNosec++
                                return true
                        }
                }
        }
        return false
}

Exclude Rule Not Working

I'm running the following command trying to exclude the hardcoded credentials finding but the finding is still being flagged.

gas -exclude=G101 ./folder/*.go

Scan without building/compiling

Hi Team,

Is it possible to scan just the code with building or compiling it.

Thanks

License?

Hello,

I love this tool! However, what's the license on this?

Broken imports, add lint, vet, fmt, etc...

Looks really bad when a code quality tool doesn't have checks running against its own PRs. Currently problem due to the following error:

# github.com/HewlettPackard/gas
src/github.com/HewlettPackard/gas/rulelist.go:18: imported and not used: "fmt"

Initialization import false positives

According to the reference,

To import a package solely for its side-effects (initialization), use the blank identifier as explicit package name:
import _ "lib/math"

However, if I run gas against the following piece of code

package main

import (
	_ "crypto/md5"
	"fmt"
)

func main() {
	fmt.Println("MD5 is not used here")
}

I will get the following output

iambrosie@xxx:~/Documents/GoCode/src/github.com/iambrosie/weakcrypto$ gas main.go
0
Results:

[main.go:4] - Use of weak cryptographic primitive (Confidence: HIGH, Severity: HIGH)
> _ "crypto/md5"

Summary:
Files: 1
Lines: 10
Nosec: 0
Issues: 1

tests of subdirectories are not skipped

I am using Mac. By looking at the source code, the '*_test.go' is in skip patterns by default.
However it doesn't work for tests in subdirectories (at least this is the case in Mac).

For example, if I run 'gas ./...' under /Users/xxx/Workspace/goprojects/src/a
then the results of the three patterns used in filelist.go are:

pathname: /Users/xxx/Workspace/goprojects/src/a/b/timeTransformer_test.go
pattern 1: _test.go - not matched
pattern 2: /Users/xxx/Workspace/goprojects/src/a/_test.go - not matched
pattern 3: **/*_test.go - not matched

I think the quick fix is to make the third pattern join 'cwd' as well
// match file suffixes ie. *_test.go if matched, _ := filepath.Match(filepath.Join(cwd, "**", pattern), pathname); matched {

I've tested then the test in subdirectories is skip.

Need a new project / org name

This project has been about for longer than ours: https://github.com/go-gas/gas so I guess we should look at coming up with a different name.

@GoASTScanner/committers Any suggestions for new project name / org name?

gas produces legitimate warnings when run against itself

Results:

[/Users/david/go/src/github.com/GoASTScanner/gas/rules/fileperms.go:39] - Errors unhandled. (Confidence: HIGH, Severity: LOW)
  > mode, _ = strconv.ParseInt(value.(string), 0, 64)


[/Users/david/go/src/github.com/GoASTScanner/gas/rules/sql.go:45] - Errors unhandled. (Confidence: HIGH, Severity: LOW)
  > str, _ := gas.GetString(start)


[/Users/david/go/src/github.com/GoASTScanner/gas/rules/sql.go:80] - Errors unhandled. (Confidence: HIGH, Severity: LOW)
  > arg, _ := gas.GetString(node.Args[0])


[/Users/david/go/src/github.com/GoASTScanner/gas/rules/tempfiles.go:32] - Errors unhandled. (Confidence: HIGH, Severity: LOW)
  > arg, _ := gas.GetString(node.Args[0])


[/Users/david/go/src/github.com/GoASTScanner/gas/tools.go:118] - Errors unhandled. (Confidence: HIGH, Severity: LOW)
  > root, _ := parser.ParseFile(fileset, filename, nil, parser.ParseComments)


[/Users/david/go/src/github.com/GoASTScanner/gas/tools.go:129] - Errors unhandled. (Confidence: HIGH, Severity: LOW)
  > pkg, _ := config.Check("main.go", fileset, []*ast.File{root}, info)


Summary:
   Files: 26
   Lines: 48936
   Nosec: 2
  Issues: 6

These all seem like things that can legitimately error so checks should probably be added. Particularly strconv.ParseInt.

panic: inconsistent import: var time.UTC *time.Location

Running gas on https://github.com/square/ghostunnel results in a panic:

cs ghostunnel master $ gas -exclude=vendor/ ./...
[gas]2016/07/25 11:42:51 Skipping /Users/cs/src/go/src/github.com/square/ghostunnel/.git
panic: inconsistent import:
    var time.UTC *time.Location
previously imported as:
    var UTC *time.Location
 [recovered]
    panic: inconsistent import:
    var time.UTC *time.Location
previously imported as:
    var UTC *time.Location


goroutine 1 [running]:
panic(0x25f640, 0xc42038d150)
    /usr/local/go/src/runtime/panic.go:500 +0x1a1
go/types.(*Checker).handleBailout(0xc42009a7e0, 0xc4203172c8)
    /usr/local/go/src/go/types/check.go:213 +0xae
panic(0x25f640, 0xc42038d150)
    /usr/local/go/src/runtime/panic.go:458 +0x243
go/internal/gcimporter.(*importer).declare(0xc42008c000, 0x3e3e20, 0xc4208be410)
    /usr/local/go/src/go/internal/gcimporter/bimport.go:173 +0x17c
go/internal/gcimporter.(*importer).obj(0xc42008c000, 0xfffffffffffffffc)
    /usr/local/go/src/go/internal/gcimporter/bimport.go:193 +0x1af
go/internal/gcimporter.BImportData(0xc4202adad0, 0xc420830000, 0x39d12, 0x3fe00, 0xc420332f31, 0x4, 0x0, 0x4, 0x0, 0x0)
    /usr/local/go/src/go/internal/gcimporter/bimport.go:86 +0x34c
go/internal/gcimporter.Import(0xc4202adad0, 0xc420332f31, 0x4, 0xc420098f00, 0x69, 0xc42027cff0, 0x0, 0x0)
    /usr/local/go/src/go/internal/gcimporter/gcimporter.go:166 +0x551
go/importer.gcimports.ImportFrom(0xc4202adad0, 0xc420332f31, 0x4, 0xc420098f00, 0x69, 0x0, 0x1, 0x2, 0x0)
    /usr/local/go/src/go/importer/importer.go:70 +0x67
go/types.(*Checker).collectObjects(0xc42009a7e0)
    /usr/local/go/src/go/types/resolver.go:191 +0x81d
go/types.(*Checker).checkFiles(0xc42009a7e0, 0xc4203173e0, 0x1, 0x1, 0x0, 0x0)
    /usr/local/go/src/go/types/check.go:225 +0xaa
go/types.(*Checker).Files(0xc42009a7e0, 0xc4203173e0, 0x1, 0x1, 0xc4202adb00, 0xfecef)
    /usr/local/go/src/go/types/check.go:218 +0x49
go/types.(*Config).Check(0xc420393d80, 0x2bb247, 0x3, 0xc42000e7c0, 0xc4203173e0, 0x1, 0x1, 0xc4200c65a0, 0x0, 0x200000003, ...)
    /usr/local/go/src/go/types/api.go:344 +0x19f
github.com/HewlettPackard/gas/core.(*Analyzer).process(0xc4200b4b60, 0xc420098f00, 0x7a, 0x0, 0x0, 0xc420098f00, 0x0)
    /Users/cs/src/go/src/github.com/HewlettPackard/gas/core/analyzer.go:85 +0x38f
github.com/HewlettPackard/gas/core.(*Analyzer).Process(0xc4200b4b60, 0xc420098f00, 0x7a, 0x3, 0xc42001e001)
    /Users/cs/src/go/src/github.com/HewlettPackard/gas/core/analyzer.go:104 +0x55
main.main.func1(0xc420098f00, 0x7a, 0x3e2ee0, 0xc4202aa750, 0x0, 0x0, 0x0, 0x0)
    /Users/cs/src/go/src/github.com/HewlettPackard/gas/main.go:122 +0x151
path/filepath.walk(0xc420098f00, 0x7a, 0x3e2ee0, 0xc4202aa750, 0xc420015bc0, 0x0, 0x0)
    /usr/local/go/src/path/filepath/path.go:349 +0x81
path/filepath.walk(0xc4200caaf0, 0x69, 0x3e2ee0, 0xc4202aa680, 0xc420015bc0, 0x0, 0x0)
    /usr/local/go/src/path/filepath/path.go:374 +0x344
path/filepath.walk(0xc4200e4f00, 0x5b, 0x3e2ee0, 0xc4202aa5b0, 0xc420015bc0, 0x0, 0x0)
    /usr/local/go/src/path/filepath/path.go:374 +0x344
path/filepath.walk(0xc4200e4c60, 0x57, 0x3e2ee0, 0xc4202aa410, 0xc420015bc0, 0x0, 0x0)
    /usr/local/go/src/path/filepath/path.go:374 +0x344
path/filepath.walk(0xc4200c63c0, 0x4c, 0x3e2ee0, 0xc4202aa340, 0xc420015bc0, 0x0, 0x0)
    /usr/local/go/src/path/filepath/path.go:374 +0x344
path/filepath.walk(0xc4208fdcc0, 0x43, 0x3e2ee0, 0xc4202abee0, 0xc420015bc0, 0x0, 0x0)
    /usr/local/go/src/path/filepath/path.go:374 +0x344
path/filepath.walk(0xc4203cdcc0, 0x38, 0x3e2ee0, 0xc4202abe10, 0xc420015bc0, 0x0, 0x0)
    /usr/local/go/src/path/filepath/path.go:374 +0x344
path/filepath.walk(0xc42000e044, 0x31, 0x3e2ee0, 0xc42006da00, 0xc420015bc0, 0x0, 0x1)
    /usr/local/go/src/path/filepath/path.go:374 +0x344
path/filepath.Walk(0xc42000e044, 0x31, 0xc420015bc0, 0x0, 0x1)
    /usr/local/go/src/path/filepath/path.go:396 +0xd5
main.main()
    /Users/cs/src/go/src/github.com/HewlettPackard/gas/main.go:128 +0xd6d

Running gas *.go appears to work fine however.

This happened on gas built from 9ce14dc, built with go1.7 beta2.

Improve Hardcoded Credentials Check via Entropy

I've disabled the hardcoded credentials check for a number of projects we're running gas on because it's too noisy and has little value. It seems like (and let me know if I'm misunderstanding) it marks everything as confidence low. I'd be interested in adding an entropy check to threshold the confidence levels; high entropy = high confidence it's a password.

Just spitballing, but I'm thinking we could do something like calculate a shannon entropy value and work out some thresholds that would map to confidence levels. This might help highlight highly random strings where there is a good chance they are a password.

Also, in light of the new NIST password guidelines, it would be interesting to supplement that with a dictionary of common passwords to match low entropy bad passwords with a higher confidence level.

Use hosted JSON guidelines for TLS evaluation

Howdy folks. Author of the Mozilla TLS guidelines here. Thanks for the great tool! I'm actually looking at using it in a number of Go projects we run.

One improvement I'd suggest to make would be to use the JSON version of the guidelines instead of hardcoding the recommendations. We host the file at https://statics.tls.security.mozilla.org/server-side-tls-conf.json You can refer to it in the code and fall back to hardcoded defaults if the retrieval fails.

As an example, here is how we use the JSON recommendations in the evaluation worker in the TLS Observatory works: https://github.com/mozilla/tls-observatory/blob/master/worker/mozillaEvaluationWorker/mozillaEvaluationWorker.go

Report use of math/rand instead of crypto/rand

It is probably unlikely that people will still make this mistake (especially since it is called out in the documentation), but we should flag the usage of math/rand instead of crypto/rand for cases when a cryptographically secure PRNG is needed.

How can I skip the vendor directory?

I tried various ways, but non of them worked. I am using git bash on windows:

$ gas $(go list ./... | grep -v vendor)
0
[gas]2016/09/10 08:44:20 GetFileAttributesEx github.com/ory-am/hydra: The system cannot find the path specified.

$ gas $(glide novendor)
0
Results:

Summary:
   Files: 0
   Lines: 0
   Nosec: 0
  Issues: 0

$ gas -skip=*vendor* ./...
0
[gas]2016/09/10 08:45:02 Skipping D:\workspace\go\src\github.com\ory-am\hydra\.docker-home
[gas]2016/09/10 08:45:02 Skipping D:\workspace\go\src\github.com\ory-am\hydra\.git
[gas]2016/09/10 08:45:02 Skipping D:\workspace\go\src\github.com\ory-am\hydra\.idea
[gas]2016/09/10 08:45:02 Skipping D:\workspace\go\src\github.com\ory-am\hydra\.vagrant
     0  *ast.Field {
     1  .  Names: []*ast.Ident (len = 2) {
     2  .  .  0: *ast.Ident {
     3  .  .  .  NamePos: D:\workspace\go\src\github.com\ory-am\hydra\vendor\github.com\fsnotify\fsnotify\integration_test.go:1229:17
...

Additionally, it doesn't look like skip works on subdirectories:

$ gas -skip=*_test.go ./...
0
[gas]2016/09/10 08:45:59 Skipping D:\workspace\go\src\github.com\ory-am\hydra\.docker-home
[gas]2016/09/10 08:45:59 Skipping D:\workspace\go\src\github.com\ory-am\hydra\.git
[gas]2016/09/10 08:45:59 Skipping D:\workspace\go\src\github.com\ory-am\hydra\.idea
[gas]2016/09/10 08:45:59 Skipping D:\workspace\go\src\github.com\ory-am\hydra\.vagrant
     0  *ast.Field {
     1  .  Names: []*ast.Ident (len = 2) {
     2  .  .  0: *ast.Ident {
     3  .  .  .  NamePos: D:\workspace\go\src\github.com\ory-am\hydra\vendor\github.com\fsnotify\fsnotify\integration_test.go:1229:17

Incorrect warning about weak crypto

Running gas 37205e9 I get:

[foo.go:411] - Use of weak random number generator (math/rand instead of crypto/rand) (Confidence: MEDIUM, Severity: HIGH)
  > rand.Read(buf)

The file in question imports math/rand as mrand and crypto/rand as rand:

import (
    "crypto/rand"
    mrand "math/rand"

So the line in question is actually using crypto/rand, and not math/rand.

README: -exclude tests* --> -exclude=tests*

Tiny issue in the readme:

gas -exclude tests* -exclude *_example.go ./...

should read

gas -exclude=tests* -exclude=*_example.go ./...

File perms error for 0644 file perms

I'm getting for a main.go file when run through gometalinter:

Expect file permissions to be 0600 or less

The perms are 0644.

Is this saying main.go should have 0 for group and other perms? If so, why is this so stringent? 0644 are the default perms, on macOS at least, and they're perfectly fine.

Revisit SQL rule logic

To pass a "go vet" run I had to tweak the logic here: 686927c

Tests still pass with this change but I think this rule needs to be refactored.

Number of lines scanned is ridiculously wrong

I've run a scan against a repo with about 300K lines of Go code and the resulting "lines of code scanned" at the end is in the 16 billion range. This is obviously pretty far off.

Benchmarks for core analyzer and matchers

Need to examine the performance profile of existing analyzer and matcher functions before expanding / rewriting existing test cases.

Support directory exclusions

I think it would be useful to support excluding directories from a scan, e.g. things like vendor/.

Running gas -exclude 'vendor/*' ./... doesn't seem to work, vendored dependencies still get scanned.

if filepath.Base(path) == "..." { works differently on Windows

I could not get the recursion to go into the sub directories on windows, the filepath.Base(path) when I use ./... as an argument will return the current directory name in Windows and not ... as expected. This is part of func getFilesToAnalyze(paths []string, excluded *fileList) []string { }. Not sure if this is a Go bug (different behavior on different platforms) or a gas bug because the filepath.Base(...) behaves differently on different OSes.

~Sean

Specifying directory trees other than $pwd

It should be possible to provide relative/or/absolute/path/.... The ellipsis to indicate the parsing of a whole subtree should not be limited to ./.... This is useful for CI systems so I don't necessarily have to change my working directory just to run gas.

[BUG] Unexpected error "undeclared name"

The latest master version with the 13th Jan changes (cc52ef5) is unexpectedly reporting the following error on one of my projects:

[gas] 2017/01/14 11:03:59 Processing "/home/user/GO/src/project/src/authrequest.go"...
[gas] 2017/01/14 11:03:59 Failed to process: "/home/user/GO/src/project/src/authrequest.go"
[gas] 2017/01/14 11:03:59 Error during type checking: "/home/user/GO/src/project/src/authrequest.go:25:22: undeclared name: SamlAuthnRequest"
[gas] 2017/01/14 11:03:59 Halting execution.

All previous gas tool versions works fine without reporting any error.
The "SamlAuthnRequest" is a defined type struct.

C FFI imports aren't working correctly

▶ cat ~/samples/native.go 
package main

// #include <stdio.h>
// #include <errno.h>
import "C"

func main() {
	C.puts(C.CString("hello"))
}

github.com/GoAstScanner/gas  master ✔                                                                                                                                                                12m  
▶ ./gas ~/samples/native.go
[gas] 2017/03/29 20:13:08 Processing "/Users/gm/samples/native.go"...
[gas] 2017/03/29 20:13:08 Error during type checking: "/Users/gm/samples/native.go:5:8: could not import C (can't find import: )"
Results:

Summary:
   Files: 1
   Lines: 9
   Nosec: 0
  Issues: 0

Hosted documentation via github pages

The skip option does not work properly

I tired the following combinations to skip the vendor folder but none of them was successful:

gas -skip vendor ./...
gas -skip vendor/* ./...
gas -skip *vendor* ./...
gas -skip vendor/*.go ./...

The vendor folder was always included into the scan.

G101: global vars and constants do not trigger rule

The rule currently only matches *ast.AssignStmt. Global var and const declarations are *ast.GenDecl so the following does not trigger the rule:

package samples

import "fmt"

var password = "admin"

func main() {
	username := "admin"
	fmt.Println("Doing something with: ", username, password)
}