Merge pull request #112 from GoASTScanner/bugfix

Report a failure and exit if type checking fails

core/analyzer.go

@@ -16,6 +16,7 @@
 package core
 
 import (
+	"fmt"
 	"go/ast"
 	"go/importer"
 	"go/parser"
@@ -123,10 +124,9 @@ func (gas *Analyzer) process(filename string, source interface{}) error {
 		}
 
 		conf := types.Config{Importer: importer.Default()}
-		gas.context.Pkg, _ = conf.Check("pkg", gas.context.FileSet, []*ast.File{root}, gas.context.Info)
+		gas.context.Pkg, err = conf.Check("pkg", gas.context.FileSet, []*ast.File{root}, gas.context.Info)
 		if err != nil {
-			gas.logger.Println("failed to check imports")
+			return fmt.Errorf(`Error during type checking: "%s"`, err)
-			return err
 		}
 
 		gas.context.Imports = NewImportInfo()

main.go

@@ -176,7 +176,7 @@ func main() {
 	// Ensure at least one file was specified
 	if flag.NArg() == 0 {
 
-		fmt.Fprintf(os.Stderr, "\nerror: FILE [FILE...] or './...' expected\n")
+		fmt.Fprintf(os.Stderr, "\nError: FILE [FILE...] or './...' expected\n")
 		flag.Usage()
 		os.Exit(1)
 	}
@@ -195,9 +195,11 @@ func main() {
 	toAnalyze := getFilesToAnalyze(flag.Args(), excluded)
 
 	for _, file := range toAnalyze {
-		logger.Printf("scanning \"%s\"\n", file)
+		logger.Printf(`Processing "%s"...`, file)
 		if err := analyzer.Process(file); err != nil {
-			logger.Fatal(err)
+			logger.Printf(`Failed to process: "%s"`, file)
+			logger.Println(err)
+			logger.Fatalf(`Halting execution.`)
 		}
 	}
 

rules/errors_test.go

@@ -32,12 +32,13 @@ func TestErrorsMulti(t *testing.T) {
 			"fmt"
 		)
 
-		func test() (val int, err error) {
+		func test() (int,error) {
 			return 0, nil
 		}
 
 		func main() {
 			v, _ := test()
+			fmt.Println(v)
 		}`, analyzer)
 
 	checkTestResults(t, issues, 1, "Errors unhandled")
@@ -130,6 +131,9 @@ func TestErrorsWhitelisted(t *testing.T) {
 			var b bytes.Buffer
 			// Default whitelist
 			nbytes, _ := b.Write([]byte("Hello "))
+			if nbytes <= 0 {
+				os.Exit(1)
+			}
 
 			// Whitelisted via configuration
 			r, _ := zlib.NewReader(&b)

rules/fileperms_test.go

@@ -31,8 +31,8 @@ func TestChmod(t *testing.T) {
 		func main() {
 			os.Chmod("/tmp/somefile", 0777)
 			os.Chmod("/tmp/someotherfile", 0600)
-			f := os.OpenFile("/tmp/thing", os.O_CREATE|os.O_WRONLY, 0666)
+			os.OpenFile("/tmp/thing", os.O_CREATE|os.O_WRONLY, 0666)
-			f := os.OpenFile("/tmp/thing", os.O_CREATE|os.O_WRONLY, 0600)
+			os.OpenFile("/tmp/thing", os.O_CREATE|os.O_WRONLY, 0600)
 		}`, analyzer)
 
 	checkTestResults(t, issues, 2, "Expect file permissions")

rules/hardcoded_credentials_test.go

@@ -90,7 +90,10 @@ func TestHardcodedConstantMulti(t *testing.T) {
 
 		import "fmt"
 
-		const username, password = "secret"
+		const (
+			username = "user"
+			password = "secret"
+		)
 
 		func main() {
 			fmt.Println("Doing something with: ", username, password)

rules/httpoxy_test.go

@@ -29,8 +29,11 @@ func TestHttpoxy(t *testing.T) {
 		package main
     import (
         "net/http/cgi"
+				"net/http"
   	)
-		func main() {}`, analyzer)
+		func main() {
+			cgi.Serve(http.FileServer(http.Dir("/usr/share/doc")))
+		}`, analyzer)
 
 	checkTestResults(t, issues, 1, "Go versions < 1.6.3 are vulnerable to Httpoxy")
 }

rules/nosec_test.go

@@ -27,13 +27,14 @@ func TestNosec(t *testing.T) {
 
 	issues := gasTestRunner(
 		`package main
-
 		import (
-    	"fmt"
+			"os"
+			"os/exec"
 		)
 
 	func main() {
-      cmd := exec.Command("sh", "-c", config.Command) // #nosec
+		cmd := exec.Command("sh", "-c", os.Getenv("BLAH")) // #nosec
+		cmd.Run()
 	}`, analyzer)
 
 	checkTestResults(t, issues, 0, "None")
@@ -46,15 +47,16 @@ func TestNosecBlock(t *testing.T) {
 
 	issues := gasTestRunner(
 		`package main
-
 		import (
-    	"fmt"
+		"os" 
+		"os/exect"
 	)
 
 	func main() {
 			// #nosec
 			if true {
-      	cmd := exec.Command("sh", "-c", config.Command)
+				cmd := exec.Command("sh", "-c", os.Getenv("BLAH"))
+				cmd.Run()
 			}
 	}`, analyzer)
 
@@ -70,11 +72,13 @@ func TestNosecIgnore(t *testing.T) {
 		`package main
 
 		import (
-    	"fmt"
+			"os"
+			"os/exec"
 		)
 
 		func main() {
-      cmd := exec.Command("sh", "-c", config.Command) // #nosec
+			cmd := exec.Command("sh", "-c", os.Args[1]) // #nosec
+			cmd.Run()
 		}`, analyzer)
 
 	checkTestResults(t, issues, 1, "Subprocess launching with variable.")

rules/rand.go

@@ -22,21 +22,23 @@ import (
 
 type WeakRand struct {
 	gas.MetaData
-	funcName    string
+	funcNames   []string
 	packagePath string
 }
 
 func (w *WeakRand) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) {
-	if _, matched := gas.MatchCallByPackage(n, c, w.packagePath, w.funcName); matched {
+	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
 		}
+	}
 
 	return nil, nil
 }
 
 func NewWeakRandCheck(conf map[string]interface{}) (gas.Rule, []ast.Node) {
 	return &WeakRand{
-		funcName:    "Read",
+		funcNames:   []string{"Read", "Int"},
 		packagePath: "math/rand",
 		MetaData: gas.MetaData{
 			Severity:   gas.High,

rules/rand_test.go

@@ -27,12 +27,13 @@ func TestRandOk(t *testing.T) {
 
 	issues := gasTestRunner(
 		`
-		package samples
+		package main 
 
 		import "crypto/rand"
 
 		func main() {
-			good, err := rand.Read(nil)
+			good, _ := rand.Read(nil)
+			println(good)
 		}`, analyzer)
 
 	checkTestResults(t, issues, 0, "Not expected to match")
@@ -45,12 +46,14 @@ func TestRandBad(t *testing.T) {
 
 	issues := gasTestRunner(
 		`
-		package samples
+		package main
 
 		import "math/rand"
 
 		func main() {
-			bad, err := rand.Read(nil)
+			bad := rand.Int()
+			println(bad)
+
 		}`, analyzer)
 
 	checkTestResults(t, issues, 1, "Use of weak random number generator (math/rand instead of crypto/rand)")
@@ -63,7 +66,7 @@ func TestRandRenamed(t *testing.T) {
 
 	issues := gasTestRunner(
 		`
-		package samples
+		package main 
 
 		import (
 			"crypto/rand"
@@ -72,8 +75,10 @@ func TestRandRenamed(t *testing.T) {
 
 
 		func main() {
-			good, err := rand.Read(nil)
+			good, _ := rand.Read(nil)
-			i := mrand.Int()
+			println(good)
+			i := mrand.Int31()
+			println(i)
 		}`, analyzer)
 
 	checkTestResults(t, issues, 0, "Not expected to match")

rules/sql_test.go

@@ -29,8 +29,8 @@ func TestSQLInjectionViaConcatenation(t *testing.T) {
         package main
         import (
                 "database/sql"
+                //_ "github.com/mattn/go-sqlite3"
                 "os"
-                _ "github.com/mattn/go-sqlite3"
         )
         func main(){
                 db, err := sql.Open("sqlite3", ":memory:")
@@ -59,7 +59,7 @@ func TestSQLInjectionViaIntepolation(t *testing.T) {
                 "database/sql"
                 "fmt"
                 "os"
-                _ "github.com/mattn/go-sqlite3"
+                //_ "github.com/mattn/go-sqlite3"
         )
         func main(){
                 db, err := sql.Open("sqlite3", ":memory:")
@@ -91,7 +91,7 @@ func TestSQLInjectionFalsePositiveA(t *testing.T) {
                 "database/sql"
                 "fmt"
                 "os"
-                _ "github.com/mattn/go-sqlite3"
+                //_ "github.com/mattn/go-sqlite3"
         )
 
         var staticQuery = "SELECT * FROM foo WHERE age < 32"
@@ -127,7 +127,7 @@ func TestSQLInjectionFalsePositiveB(t *testing.T) {
                 "database/sql"
                 "fmt"
                 "os"
-                _ "github.com/mattn/go-sqlite3"
+                //_ "github.com/mattn/go-sqlite3"
         )
 
         var staticQuery = "SELECT * FROM foo WHERE age < 32"
@@ -163,7 +163,7 @@ func TestSQLInjectionFalsePositiveC(t *testing.T) {
                 "database/sql"
                 "fmt"
                 "os"
-                _ "github.com/mattn/go-sqlite3"
+                //_ "github.com/mattn/go-sqlite3"
         )
 
         var staticQuery = "SELECT * FROM foo WHERE age < "
@@ -199,7 +199,7 @@ func TestSQLInjectionFalsePositiveD(t *testing.T) {
                 "database/sql"
                 "fmt"
                 "os"
-                _ "github.com/mattn/go-sqlite3"
+                //_ "github.com/mattn/go-sqlite3"
         )
 
 				const age = "32"

rules/subproc_test.go

@@ -58,11 +58,12 @@ func TestSubprocessVar(t *testing.T) {
 
     import (
     	"log"
+			"os"
     	"os/exec"
     )
 
     func main() {
-      run := "sleep" + someFunc()
+      run := "sleep" + os.Getenv("SOMETHING")
     	cmd := exec.Command(run, "5")
     	err := cmd.Start()
     	if err != nil {
@@ -112,8 +113,7 @@ func TestSubprocessSyscall(t *testing.T) {
     package main
 
     import (
-    	"log"
+       "syscall"
-    	"os/exec"
     )
 
     func main() {

rules/tls_test.go

@@ -124,7 +124,7 @@ func TestInsecureCipherSuite(t *testing.T) {
         func main() {
         	tr := &http.Transport{
         		TLSClientConfig: &tls.Config{CipherSuites: []uint16{
-                                tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_DERP,
+                                tls.TLS_RSA_WITH_RC4_128_SHA,
                                 tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
                                 },},
         	}
@@ -136,5 +136,5 @@ func TestInsecureCipherSuite(t *testing.T) {
         }
         `, analyzer)
 
-	checkTestResults(t, issues, 1, "TLS Bad Cipher Suite: TLS_ECDHE_RSA_WITH_AES_128_GCM_DERP")
+	checkTestResults(t, issues, 1, "TLS Bad Cipher Suite: TLS_RSA_WITH_RC4_128_SHA")
 }