From 5a131be2ec212523a60df32fc8e6c099aa7b241f Mon Sep 17 00:00:00 2001
From: Nanik <nanikjava@gmail.com>
Date: Mon, 16 Aug 2021 19:31:51 +1000
Subject: [PATCH] fix: add more rules for G204 (#677)

* fix: add more rules for G204

* fix: add extra test and comment
---
 rules/subproc.go    | 28 ++++++++++++++++++++++++++--
 testutils/source.go | 31 ++++++++++++++++++++++++++++++-
 2 files changed, 56 insertions(+), 3 deletions(-)

diff --git a/rules/subproc.go b/rules/subproc.go
index 4110d9d..45481bc 100644
--- a/rules/subproc.go
+++ b/rules/subproc.go
@@ -48,8 +48,32 @@ func (r *subprocess) Match(n ast.Node, c *gosec.Context) (*gosec.Issue, error) {
 		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) {
-					return gosec.NewIssue(c, n, r.ID(), "Subprocess launched with variable", gosec.Medium, gosec.High), nil
+
+				// need to cast and check whether it is for a variable ?
+				_, variable := obj.(*types.Var)
+
+				// .. indeed it is a variable then processing is different than a normal
+				// field assignment
+				if variable {
+					switch ident.Obj.Decl.(type) {
+					case *ast.AssignStmt:
+						_, assignment := ident.Obj.Decl.(*ast.AssignStmt)
+						if variable && assignment {
+							if !gosec.TryResolve(ident, c) {
+								return gosec.NewIssue(c, n, r.ID(), "Subprocess launched with variable", gosec.Medium, gosec.High), nil
+							}
+						}
+					case *ast.Field:
+						_, field := ident.Obj.Decl.(*ast.Field)
+						if variable && field {
+							// check if the variable exist in the scope
+							vv, vvok := obj.(*types.Var)
+
+							if vvok && vv.Parent().Lookup(ident.Name) == nil {
+								return gosec.NewIssue(c, n, r.ID(), "Subprocess launched with variable", gosec.Medium, gosec.High), nil
+							}
+						}
+					}
 				}
 			} else if !gosec.TryResolve(arg, c) {
 				// the arg is not a constant or a variable but instead a function call or os.Args[i]
diff --git a/testutils/source.go b/testutils/source.go
index b336a48..e741794 100644
--- a/testutils/source.go
+++ b/testutils/source.go
@@ -1369,7 +1369,36 @@ func RunCmd(command string) {
 
 func main() {
 	RunCmd("sleep")
-}`}, 1, gosec.NewConfig()},
+}`}, 0, gosec.NewConfig()},
+		{[]string{`
+package main
+
+import (
+	"log"
+	"os/exec"
+)
+
+func RunCmd(a string, c string) {
+	cmd := exec.Command(c)
+	err := cmd.Start()
+	if err != nil {
+		log.Fatal(err)
+	}
+	log.Printf("Waiting for command to finish...")
+	err = cmd.Wait()
+
+	cmd = exec.Command(a)
+	err = cmd.Start()
+	if err != nil {
+		log.Fatal(err)
+	}
+	log.Printf("Waiting for command to finish...")
+	err = cmd.Wait()
+}
+
+func main() {
+	RunCmd("ll", "ls")
+}`}, 0, gosec.NewConfig()},
 		{[]string{`
 // syscall.Exec function called with harcoded arguments
 // shouldn't be consider as a command injection