Skip to content

Commit 6e10f39

Browse files
geoffw0geoffw0
authored
Merge pull request #319 from raulgarciamsft/users/raulga/c6277
C++ : NULL application name with an unquoted path in call to CreateProcess
2 parents 749206a + 7ab723a commit 6e10f39

File tree

8 files changed

+635
-0
lines changed

8 files changed

+635
-0
lines changed

‎cpp/config/suites/security/cwe-428‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
# CWE-428: Unquoted Search Path or Element
2+
+ semmlecode-cpp-queries/Security/CWE/CWE-428/UnsafeCreateProcessCall.ql: /CWE/CWE-428
3+
@name NULL application name with an unquoted path in call to CreateProcess (CWE-428)

‎cpp/config/suites/security/default‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
@import "cwe-327"
2020
@import "cwe-367"
2121
@import "cwe-416"
22+
@import "cwe-428"
2223
@import "cwe-457"
2324
@import "cwe-468"
2425
@import "cwe-676"

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
STARTUPINFOW si;
2+
PROCESS_INFORMATION pi;
3+
4+
// ...
5+
6+
CreateProcessW( // BUG
7+
NULL, // lpApplicationName
8+
(LPWSTR)L"C:\\Program Files\\MyApp", // lpCommandLine
9+
NULL, NULL, FALSE, 0, NULL, NULL, &si, &pi);
10+
11+
// ...

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

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>This query indicates that there is a call to a function of the <code>CreateProcess*</code> family of functions, which introduces a security vulnerability.</p>
8+
</overview>
9+
10+
<recommendation>
11+
<p>Do not use <code>NULL</code> for the <code>lpApplicationName</code> argument to the <code>CreateProcess*</code> function.</p>
12+
<p>If you pass <code>NULL</code> for <code>lpApplicationName</code>, use quotation marks around the executable path in <code>lpCommandLine</code>.</p>
13+
</recommendation>
14+
15+
<example>
16+
<p>In the following example, <code>CreateProcessW</code> is called with a <code>NULL</code> value for <code>lpApplicationName</code>,
17+
and the value for <code>lpCommandLine</code> that represent the application path is not quoted and has spaces in it.</p>
18+
<p>If an attacker has access to the file system, they can elevate privileges by creating a file such as <code>C:\Program.exe</code> that will be executed instead of the intended application.</p>
19+
<sample src="UnsafeCreateProcessCall.cpp" />
20+
21+
<p>To fix this issue, specify a valid string for <code>lpApplicationName</code>, or quote the path for <code>lpCommandLine</code>. For example:</p>
22+
<p><code>(LPWSTR)L"\"C:\\Program Files\\MyApp\"", // lpCommandLine</code></p>
23+
</example>
24+
25+
<references>
26+
<li>
27+
<a href="https://docs.microsoft.com/en-us/windows/desktop/api/processthreadsapi/nf-processthreadsapi-createprocessa">CreateProcessA function (Microsoft documentation).</a>
28+
</li>
29+
<li>
30+
<a href="https://docs.microsoft.com/en-us/windows/desktop/api/processthreadsapi/nf-processthreadsapi-createprocessw">CreateProcessW function (Microsoft documentation).</a>
31+
</li>
32+
<li>
33+
<a href="https://docs.microsoft.com/en-us/windows/desktop/api/processthreadsapi/nf-processthreadsapi-createprocessasusera">CreateProcessAsUserA function (Microsoft documentation).</a>
34+
</li>
35+
<li>
36+
<a href="https://docs.microsoft.com/en-us/windows/desktop/api/processthreadsapi/nf-processthreadsapi-createprocessasuserw">CreateProcessAsUserW function (Microsoft documentation).</a>
37+
</li>
38+
<li>
39+
<a href="https://docs.microsoft.com/en-us/windows/desktop/api/winbase/nf-winbase-createprocesswithlogonw">CreateProcessWithLogonW function (Microsoft documentation).</a>
40+
</li>
41+
<li>
42+
<a href="https://docs.microsoft.com/en-us/windows/desktop/api/winbase/nf-winbase-createprocesswithtokenw">CreateProcessWithTokenW function (Microsoft documentation).</a>
43+
</li>
44+
</references>
45+
46+
</qhelp>

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

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
/**
2+
* @name NULL application name with an unquoted path in call to CreateProcess
3+
* @description Calling a function of the CreateProcess* family of functions, where the path contains spaces, introduces a security vulnerability.
4+
* @id cpp/unsafe-create-process-call
5+
* @kind problem
6+
* @problem.severity error
7+
* @precision medium
8+
* @msrc.severity important
9+
* @tags security
10+
* external/cwe/cwe-428
11+
* external/microsoft/C6277
12+
*/
13+
14+
import cpp
15+
import semmle.code.cpp.dataflow.DataFlow
16+
import semmle.code.cpp.dataflow.DataFlow2
17+
18+
predicate isCreateProcessFunction(FunctionCall call, int applicationNameIndex, int commandLineIndex) {
19+
(
20+
call.getTarget().hasGlobalName("CreateProcessA")
21+
and applicationNameIndex = 0
22+
and commandLineIndex = 1
23+
) or (
24+
call.getTarget().hasGlobalName("CreateProcessW")
25+
and applicationNameIndex = 0
26+
and commandLineIndex = 1
27+
) or (
28+
call.getTarget().hasGlobalName("CreateProcessWithTokenW")
29+
and applicationNameIndex = 2
30+
and commandLineIndex = 3
31+
) or (
32+
call.getTarget().hasGlobalName("CreateProcessWithLogonW")
33+
and applicationNameIndex = 4
34+
and commandLineIndex = 5
35+
) or (
36+
call.getTarget().hasGlobalName("CreateProcessAsUserA")
37+
and applicationNameIndex = 1
38+
and commandLineIndex = 2
39+
) or (
40+
call.getTarget().hasGlobalName("CreateProcessAsUserW")
41+
and applicationNameIndex = 1
42+
and commandLineIndex = 2
43+
)
44+
}
45+
/**
46+
* A function call to CreateProcess (either wide-char or single byte string versions)
47+
*/
48+
class CreateProcessFunctionCall extends FunctionCall {
49+
CreateProcessFunctionCall() {
50+
isCreateProcessFunction( this, _, _)
51+
}
52+
53+
int getApplicationNameArgumentId() {
54+
isCreateProcessFunction( this, result, _)
55+
}
56+
57+
int getCommandLineArgumentId() {
58+
isCreateProcessFunction( this, _, result)
59+
}
60+
}
61+
62+
/**
63+
* Dataflow that detects a call to CreateProcess with a NULL value for lpApplicationName argument
64+
*/
65+
class NullAppNameCreateProcessFunctionConfiguration extends DataFlow::Configuration {
66+
NullAppNameCreateProcessFunctionConfiguration() {
67+
this = "NullAppNameCreateProcessFunctionConfiguration"
68+
}
69+
70+
override predicate isSource(DataFlow::Node source) {
71+
nullValue(source.asExpr())
72+
}
73+
74+
override predicate isSink(DataFlow::Node sink) {
75+
exists(
76+
CreateProcessFunctionCall call, Expr val |
77+
val = sink.asExpr() |
78+
val = call.getArgument(call.getApplicationNameArgumentId())
79+
)
80+
}
81+
}
82+
83+
/**
84+
* Dataflow that detects a call to CreateProcess with an unquoted commandLine argument
85+
*/
86+
class QuotedCommandInCreateProcessFunctionConfiguration extends DataFlow2::Configuration {
87+
QuotedCommandInCreateProcessFunctionConfiguration() {
88+
this = "QuotedCommandInCreateProcessFunctionConfiguration"
89+
}
90+
91+
override predicate isSource(DataFlow2::Node source) {
92+
exists( string s |
93+
s = source.asExpr().getValue().toString()
94+
and
95+
not isQuotedOrNoSpaceApplicationNameOnCmd(s)
96+
)
97+
}
98+
99+
override predicate isSink(DataFlow2::Node sink) {
100+
exists(
101+
CreateProcessFunctionCall call, Expr val |
102+
val = sink.asExpr() |
103+
val = call.getArgument(call.getCommandLineArgumentId())
104+
)
105+
}
106+
}
107+
108+
bindingset[s]
109+
predicate isQuotedOrNoSpaceApplicationNameOnCmd(string s){
110+
s.regexpMatch("\"([^\"])*\"(\\s|.)*") // The first element (path) is quoted
111+
or
112+
s.regexpMatch("[^\\s]+") // There are no spaces in the string
113+
}
114+
115+
from CreateProcessFunctionCall call, string msg1, string msg2
116+
where
117+
exists( Expr source, Expr appName,
118+
NullAppNameCreateProcessFunctionConfiguration nullAppConfig |
119+
appName = call.getArgument(call.getApplicationNameArgumentId())
120+
and nullAppConfig.hasFlow(DataFlow2::exprNode(source), DataFlow2::exprNode(appName))
121+
and msg1 = call.toString() + " with lpApplicationName == NULL (" + appName + ")"
122+
)
123+
and
124+
exists( Expr source, Expr cmd,
125+
QuotedCommandInCreateProcessFunctionConfiguration quotedConfig |
126+
cmd = call.getArgument(call.getCommandLineArgumentId())
127+
and quotedConfig.hasFlow(DataFlow2::exprNode(source), DataFlow2::exprNode(cmd))
128+
and msg2 = " and with an unquoted lpCommandLine (" + cmd + ") introduces a security vulnerability if the path contains spaces."
129+
)
130+
select call, msg1 + " " + msg2

0 commit comments

Comments
 (0)