Skip to content

Commit 22d5480

Browse files
Removed one false-positive scenario (no space on lpCommandLine)
Improved the query to avoid multiple calls to hasGlobalName Fixed typos Simplified the test case file
1 parent cd5e788 commit 22d5480

File tree

3 files changed

+201
-280
lines changed

3 files changed

+201
-280
lines changed

cpp/ql/src/Security/CWE/CWE-428/UnsafeCreateProcessCall.ql

Lines changed: 35 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -14,55 +14,47 @@ import cpp
1414
import semmle.code.cpp.dataflow.DataFlow
1515
import semmle.code.cpp.dataflow.DataFlow2
1616

17+
predicate isCreateProcessFunction(FunctionCall call, int applicationNameIndex, int commandLineIndex) {
18+
(
19+
call.getTarget().hasGlobalName("CreateProcessA")
20+
and applicationNameIndex = 0
21+
and commandLineIndex = 1
22+
) or (
23+
call.getTarget().hasGlobalName("CreateProcessW")
24+
and applicationNameIndex = 0
25+
and commandLineIndex = 1
26+
) or (
27+
call.getTarget().hasGlobalName("CreateProcessWithTokenW")
28+
and applicationNameIndex = 2
29+
and commandLineIndex = 3
30+
) or (
31+
call.getTarget().hasGlobalName("CreateProcessWithLogonW")
32+
and applicationNameIndex = 4
33+
and commandLineIndex = 5
34+
) or (
35+
call.getTarget().hasGlobalName("CreateProcessAsUserA")
36+
and applicationNameIndex = 1
37+
and commandLineIndex = 2
38+
) or (
39+
call.getTarget().hasGlobalName("CreateProcessAsUserW")
40+
and applicationNameIndex = 1
41+
and commandLineIndex = 2
42+
)
43+
}
1744
/**
1845
* A function call to CreateProcess (either wide-char or single byte string versions)
1946
*/
2047
class CreateProcessFunctionCall extends FunctionCall {
2148
CreateProcessFunctionCall() {
22-
(
23-
this.getTarget().hasGlobalName("CreateProcessA") or
24-
this.getTarget().hasGlobalName("CreateProcessW") or
25-
this.getTarget().hasGlobalName("CreateProcessWithTokenW") or
26-
this.getTarget().hasGlobalName("CreateProcessWithLogonW") or
27-
this.getTarget().hasGlobalName("CreateProcessAsUserA") or
28-
this.getTarget().hasGlobalName("CreateProcessAsUserW")
29-
)
49+
isCreateProcessFunction( this, _, _)
3050
}
3151

3252
int getApplicationNameArgumentId() {
33-
if(
34-
this.getTarget().hasGlobalName("CreateProcessA") or
35-
this.getTarget().hasGlobalName("CreateProcessW")
36-
) then ( result = 0 )
37-
else if (
38-
this.getTarget().hasGlobalName("CreateProcessWithTokenW")
39-
) then ( result = 2 )
40-
else if (
41-
this.getTarget().hasGlobalName("CreateProcessWithLogonW")
42-
) then ( result = 4 )
43-
else if(
44-
this.getTarget().hasGlobalName("CreateProcessAsUserA") or
45-
this.getTarget().hasGlobalName("CreateProcessAsUserW")
46-
) then ( result = 1 )
47-
else (result = -1 )
53+
isCreateProcessFunction( this, result, _)
4854
}
4955

5056
int getCommandLineArgumentId() {
51-
if(
52-
this.getTarget().hasGlobalName("CreateProcessA") or
53-
this.getTarget().hasGlobalName("CreateProcessW")
54-
) then ( result = 1 )
55-
else if (
56-
this.getTarget().hasGlobalName("CreateProcessWithTokenW")
57-
) then ( result = 3 )
58-
else if (
59-
this.getTarget().hasGlobalName("CreateProcessWithLogonW")
60-
) then ( result = 5 )
61-
else if(
62-
this.getTarget().hasGlobalName("CreateProcessAsUserA") or
63-
this.getTarget().hasGlobalName("CreateProcessAsUserW")
64-
) then ( result = 2 )
65-
else (result = -1 )
57+
isCreateProcessFunction( this, _, result)
6658
}
6759
}
6860

@@ -99,7 +91,7 @@ class QuotedCommandInCreateProcessFunctionConfiguration extends DataFlow2::Confi
9991
exists( string s |
10092
s = source.asExpr().getValue().toString()
10193
and
102-
not isQuotedApplicationNameOnCmd(s)
94+
not isQuotedOrNoSpaceApplicationNameOnCmd(s)
10395
)
10496
}
10597

@@ -113,8 +105,10 @@ class QuotedCommandInCreateProcessFunctionConfiguration extends DataFlow2::Confi
113105
}
114106

115107
bindingset[s]
116-
predicate isQuotedApplicationNameOnCmd(string s){
117-
s.regexpMatch("\"([^\"])*\"(\\s|.)*")
108+
predicate isQuotedOrNoSpaceApplicationNameOnCmd(string s){
109+
s.regexpMatch("\"([^\"])*\"(\\s|.)*") // The first element (path) is quoted
110+
or
111+
s.regexpMatch("[^\\s]+") // There are no spaces in the string
118112
}
119113

120114
from CreateProcessFunctionCall call, string msg1, string msg2

0 commit comments

Comments
 (0)