Fix the subproc rule to handle correctly the CommandContext check

In this case, we need to skip the first argument because it is the context. Signed-off-by: Cosmin Cojocar <cosmin.cojocar@gmx.ch>
knightyzl · Mar 13, 2020 · cf25904 · cf25904
1 parent f97f861
commit cf25904
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 6 deletions.
diff --git a/rules/subproc.go b/rules/subproc.go
@@ -41,7 +41,11 @@ func (r *subprocess) ID() string {
 // syscall.Exec("echo", "foobar" + tainted)
 func (r *subprocess) Match(n ast.Node, c *gosec.Context) (*gosec.Issue, error) {
 	if node := r.ContainsPkgCallExpr(n, c, false); node != nil {
-		for _, arg := range node.Args {
+		args := node.Args
+		if r.isContext(n, c) {
+			args = args[1:]
+		}
+		for _, arg := range args {
 			if ident, ok := arg.(*ast.Ident); ok {
 				obj := c.Info.ObjectOf(ident)
 				if _, ok := obj.(*types.Var); ok && !gosec.TryResolve(ident, c) {
@@ -56,6 +60,19 @@ func (r *subprocess) Match(n ast.Node, c *gosec.Context) (*gosec.Issue, error) {
 	return nil, nil
 }
 
+// isContext checks whether or not the node is a CommandContext call or not
+// Thi is requried in order to skip the first argument from the check.
+func (r *subprocess) isContext(n ast.Node, ctx *gosec.Context) bool {
+	selector, indent, err := gosec.GetCallInfo(n, ctx)
+	if err != nil {
+		return false
+	}
+	if selector == "exec" && indent == "CommandContext" {
+		return true
+	}
+	return false
+}
+
 // NewSubproc detects cases where we are forking out to an external process
 func NewSubproc(id string, conf gosec.Config) (gosec.Rule, []ast.Node) {
 	rule := &subprocess{gosec.MetaData{ID: id}, gosec.NewCallList()}

diff --git a/testutils/source.go b/testutils/source.go
@@ -980,21 +980,19 @@ func main() {
 
 	// SampleCodeG204 - Subprocess auditing
 	SampleCodeG204 = []CodeSample{{[]string{`
-// Calling any function which starts a new process
-// with a function call as an argument is considered a command injection
 package main
 import (
 	"log"
 	"os/exec"
 	"context"
 )
 func main() {
-	err := exec.CommandContext(context.Background(), "sleep", "5").Run()
+	err := exec.CommandContext(context.Background(), "git", "rev-parse", "--show-toplavel").Run()
  	if err != nil {
 		log.Fatal(err)
 	}
   	log.Printf("Command finished with error: %v", err)
-}`}, 1, gosec.NewConfig()}, {[]string{`
+}`}, 0, gosec.NewConfig()}, {[]string{`
 // Calling any function which starts a new process with using
 // command line arguments as it's arguments is considered dangerous
 package main
@@ -1004,7 +1002,7 @@ import (
 	"os/exec"
 )
 func main() {
-	err := exec.CommandContext(os.Args[0], "sleep", "5").Run()
+	err := exec.CommandContext(context.Background(), os.Args[0], "5").Run()
 	if err != nil {
 		log.Fatal(err)
 	}