Skip to content

Commit 9aeebc6

Browse files
committed
update the QHelp to add a "--" example
1 parent ea2b73b commit 9aeebc6

File tree

4 files changed

+61
-28
lines changed

4 files changed

+61
-28
lines changed

go/ql/src/Security/CWE-078/CommandInjection.qhelp

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,18 +12,17 @@ a malicious user may be able to run commands to exfiltrate data or compromise th
1212

1313
<recommendation>
1414
<p>
15-
If possible, use hard-coded string literals to specify the command to run, and avoid using
16-
shell string interpreters such as <code>sh -c</code> to execute shell commands.
15+
Whenever possible, use hard-coded string literals for commands and avoid shell
16+
string interpreters like <code>sh -c</code>.
1717
</p>
1818
<p>
1919
If given arguments as a single string, avoid simply splitting the string on
2020
whitespace. Arguments may contain quoted whitespace, causing them to split into
2121
multiple arguments.
2222
</p>
2323
<p>
24-
If this is not possible, then add sanitization code to verify that the user input is
25-
safe before using it, thereby avoiding characters that can change the meaning of the
26-
command such as spaces and quotes.
24+
If this is not possible, sanitize user input to avoid characters like spaces and
25+
various kinds of quotes that can alter the meaning of the command.
2726
</p>
2827
</recommendation>
2928

@@ -34,12 +33,11 @@ handler in a web application, whose parameter <code>req</code> contains the requ
3433
</p>
3534
<sample src="examples/CommandInjection.go"/>
3635
<p>
37-
The handler extracts the name of an image file from the request object, and then runs a command
38-
to process the image. The command is constructed by concatenating the image path and the output path,
39-
and then running it with <code>sh -c</code>. This can cause a command-injection vulnerability.
36+
The handler extracts the image file name from the request and uses the image name to construct a
37+
shell command that is executed using <code>`sh -c`</code>, which can lead to command injection.
4038
</p>
4139
<p>
42-
It's better to avoid shell strings by using the <code>exec.Command</code> function directly,
40+
It's better to avoid shell commands by using the <code>exec.Command</code> function directly,
4341
as shown in the following example:
4442
</p>
4543
<sample src="examples/CommandInjectionGood.go"/>
@@ -48,6 +46,15 @@ Alternatively, a regular expression can be used to ensure that the image name is
4846
in a shell command:
4947
</p>
5048
<sample src="examples/CommandInjectionGood2.go"/>
49+
<p>
50+
Some commands, like <code>git</code>, can indirectly execute commands if an attacker specifies
51+
the flags given to the command.
52+
</p>
53+
<p>
54+
To mitigate this risk, either add a <code>--</code> argument to ensure subsequent arguments are
55+
not interpreted as flags, or verify that the argument does not start with <code>"--"</code>.
56+
</p>
57+
<sample src="examples/CommandInjectionGood3.go"/>
5158
</example>
5259
<references>
5360
<li>

go/ql/src/Security/CWE-078/examples/CommandInjection.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@ import (
77

88
func handler(req *http.Request) {
99
imageName := req.URL.Query()["imageName"][0]
10-
outputPath = "/tmp/output.svg"
10+
outputPath := "/tmp/output.svg"
1111
cmd := exec.Command("sh", "-c", fmt.Sprintf("imagetool %s > %s", imageName, outputPath))
1212
cmd.Run()
13-
// ...
13+
// ...
1414
}
Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,28 @@
11
package main
22

33
import (
4-
"log"
5-
"net/http"
6-
"os"
7-
"os/exec"
4+
"log"
5+
"net/http"
6+
"os"
7+
"os/exec"
88
)
99

1010
func handler(req *http.Request) {
11-
imageName := req.URL.Query()["imageName"][0]
12-
outputPath := "/tmp/output.svg"
11+
imageName := req.URL.Query()["imageName"][0]
12+
outputPath := "/tmp/output.svg"
1313

14-
// Create the output file
15-
outfile, err := os.Create(outputPath)
16-
if err != nil {
17-
log.Fatal(err)
18-
}
19-
defer outfile.Close()
14+
// Create the output file
15+
outfile, err := os.Create(outputPath)
16+
if err != nil {
17+
log.Fatal(err)
18+
}
19+
defer outfile.Close()
2020

21-
// Prepare the command
22-
cmd := exec.Command("imagetool", imageName)
21+
// Prepare the command
22+
cmd := exec.Command("imagetool", imageName)
2323

24-
// Set the output to our file
25-
cmd.Stdout = outfile
24+
// Set the output to our file
25+
cmd.Stdout = outfile
2626

27-
cmd.Run()
27+
cmd.Run()
2828
}
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
package main
2+
3+
import (
4+
"log"
5+
"net/http"
6+
"os/exec"
7+
"strings"
8+
)
9+
10+
func handler(req *http.Request) {
11+
repoURL := req.URL.Query()["repoURL"][0]
12+
outputPath := "/tmp/repo"
13+
14+
// Sanitize the repoURL to ensure it does not start with "--"
15+
if strings.HasPrefix(repoURL, "--") {
16+
log.Fatal("Invalid repository URL")
17+
return
18+
}
19+
20+
// Or: add "--" to ensure that the repoURL is not interpreted as a flag
21+
cmd := exec.Command("git", "clone", "--", repoURL, outputPath)
22+
err := cmd.Run()
23+
if err != nil {
24+
log.Fatal(err)
25+
}
26+
}

0 commit comments

Comments
 (0)