Skip to content

Commit cf25904

Browse files
ccojocarCosmin Cojocar
authored andcommitted
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 <[email protected]>
1 parent f97f861 commit cf25904

File tree

2 files changed

+21
-6
lines changed

2 files changed

+21
-6
lines changed

rules/subproc.go

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,11 @@ func (r *subprocess) ID() string {
4141
// syscall.Exec("echo", "foobar" + tainted)
4242
func (r *subprocess) Match(n ast.Node, c *gosec.Context) (*gosec.Issue, error) {
4343
if node := r.ContainsPkgCallExpr(n, c, false); node != nil {
44-
for _, arg := range node.Args {
44+
args := node.Args
45+
if r.isContext(n, c) {
46+
args = args[1:]
47+
}
48+
for _, arg := range args {
4549
if ident, ok := arg.(*ast.Ident); ok {
4650
obj := c.Info.ObjectOf(ident)
4751
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) {
5660
return nil, nil
5761
}
5862

63+
// isContext checks whether or not the node is a CommandContext call or not
64+
// Thi is requried in order to skip the first argument from the check.
65+
func (r *subprocess) isContext(n ast.Node, ctx *gosec.Context) bool {
66+
selector, indent, err := gosec.GetCallInfo(n, ctx)
67+
if err != nil {
68+
return false
69+
}
70+
if selector == "exec" && indent == "CommandContext" {
71+
return true
72+
}
73+
return false
74+
}
75+
5976
// NewSubproc detects cases where we are forking out to an external process
6077
func NewSubproc(id string, conf gosec.Config) (gosec.Rule, []ast.Node) {
6178
rule := &subprocess{gosec.MetaData{ID: id}, gosec.NewCallList()}

testutils/source.go

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -980,21 +980,19 @@ func main() {
980980

981981
// SampleCodeG204 - Subprocess auditing
982982
SampleCodeG204 = []CodeSample{{[]string{`
983-
// Calling any function which starts a new process
984-
// with a function call as an argument is considered a command injection
985983
package main
986984
import (
987985
"log"
988986
"os/exec"
989987
"context"
990988
)
991989
func main() {
992-
err := exec.CommandContext(context.Background(), "sleep", "5").Run()
990+
err := exec.CommandContext(context.Background(), "git", "rev-parse", "--show-toplavel").Run()
993991
if err != nil {
994992
log.Fatal(err)
995993
}
996994
log.Printf("Command finished with error: %v", err)
997-
}`}, 1, gosec.NewConfig()}, {[]string{`
995+
}`}, 0, gosec.NewConfig()}, {[]string{`
998996
// Calling any function which starts a new process with using
999997
// command line arguments as it's arguments is considered dangerous
1000998
package main
@@ -1004,7 +1002,7 @@ import (
10041002
"os/exec"
10051003
)
10061004
func main() {
1007-
err := exec.CommandContext(os.Args[0], "sleep", "5").Run()
1005+
err := exec.CommandContext(context.Background(), os.Args[0], "5").Run()
10081006
if err != nil {
10091007
log.Fatal(err)
10101008
}

0 commit comments

Comments
 (0)