Skip to content

Commit 85283d6

Browse files
raulgarciamsftraulgarciamsft
committed
C++ : NULL application name with an unquoted path in call to CreateProcess
Calling a function of the CreatePorcess* family of functions, which may result in a security vulnerability if the path contains spaces.
1 parent 54493eb commit 85283d6

File tree

9 files changed

+714
-0
lines changed

9 files changed

+714
-0
lines changed

‎.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,4 @@
1313
/.vs/ql/v15/Browse.VC.db
1414
/.vs/ProjectSettings.json
1515

16+
/.vs/ql/v15/.suo

‎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
@@ -18,6 +18,7 @@
1818
@import "cwe-327"
1919
@import "cwe-367"
2020
@import "cwe-416"
21+
@import "cwe-428"
2122
@import "cwe-457"
2223
@import "cwe-468"
2324
@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>CreatePorcess*</code> family of functions, which may result in a security vulnerability if the path contains spaces.</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 NULL value for <code>lpApplicationName</code>,
17+
and the value for <code>lpCommandLine</code> that represent the application path is not quoted and has spaces int.</p>
18+
<p>If an attacker has access to the file system, it is possible to elevate privileges by creating a file such as "C:\Program.exe" 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: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
/**
2+
* @name NULL application name with an unquoted path in call to CreateProcess
3+
* @description Calling a function of the CreatePorcess* family of functions, which may result in a security vulnerability if the path contains spaces.
4+
* @id cpp/unsafe-create-process-call
5+
* @kind problem
6+
* @problem.severity error
7+
* @precision medium
8+
* @tags security
9+
* external/cwe/cwe-428
10+
* external/microsoft/C6277
11+
*/
12+
13+
import cpp
14+
import semmle.code.cpp.dataflow.DataFlow
15+
import semmle.code.cpp.dataflow.DataFlow2
16+
17+
/**
18+
* A function call to CreateProcess (either wide-char or single byte string versions)
19+
*/
20+
class CreateProcessFunctionCall extends FunctionCall {
21+
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+
)
30+
}
31+
32+
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 )
48+
}
49+
50+
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 )
66+
}
67+
}
68+
69+
/**
70+
* Dataflow that detects a call to CreateProcess with a NULL value for lpApplicationName argument
71+
*/
72+
class NullAppNameCreateProcessFunctionConfiguration extends DataFlow::Configuration {
73+
NullAppNameCreateProcessFunctionConfiguration() {
74+
this = "NullAppNameCreateProcessFunctionConfiguration"
75+
}
76+
77+
override predicate isSource(DataFlow::Node source) {
78+
nullValue(source.asExpr())
79+
}
80+
81+
override predicate isSink(DataFlow::Node sink) {
82+
exists(
83+
CreateProcessFunctionCall call, Expr val |
84+
val = sink.asExpr() |
85+
val = call.getArgument(call.getApplicationNameArgumentId())
86+
)
87+
}
88+
}
89+
90+
/**
91+
* Dataflow that detects a call to CreateProcess with an unquoted commandLine argument
92+
*/
93+
class QuotedCommandInCreateProcessFunctionConfiguration extends DataFlow2::Configuration {
94+
QuotedCommandInCreateProcessFunctionConfiguration() {
95+
this = "QuotedCommandInCreateProcessFunctionConfiguration"
96+
}
97+
98+
override predicate isSource(DataFlow2::Node source) {
99+
exists( string s |
100+
s = source.asExpr().getValue().toString()
101+
and
102+
not isQuotedApplicationNameOnCmd(s)
103+
)
104+
}
105+
106+
override predicate isSink(DataFlow2::Node sink) {
107+
exists(
108+
CreateProcessFunctionCall call, Expr val |
109+
val = sink.asExpr() |
110+
val = call.getArgument(call.getCommandLineArgumentId())
111+
)
112+
}
113+
}
114+
115+
bindingset[s]
116+
predicate isQuotedApplicationNameOnCmd(string s){
117+
s.regexpMatch("\"([^\"])*\"(\\s|.)*")
118+
}
119+
120+
from CreateProcessFunctionCall call, string msg1, string msg2
121+
where
122+
exists( Expr source, Expr appName,
123+
NullAppNameCreateProcessFunctionConfiguration nullAppConfig |
124+
appName = call.getArgument(call.getApplicationNameArgumentId())
125+
and nullAppConfig.hasFlow(DataFlow2::exprNode(source), DataFlow2::exprNode(appName))
126+
and msg1 = call.toString() + " with lpApplicationName == NULL (" + appName + ")"
127+
)
128+
and
129+
exists( Expr source, Expr cmd,
130+
QuotedCommandInCreateProcessFunctionConfiguration quotedConfig |
131+
cmd = call.getArgument(call.getCommandLineArgumentId())
132+
and quotedConfig.hasFlow(DataFlow2::exprNode(source), DataFlow2::exprNode(cmd))
133+
and msg2 = " and with an unquoted lpCommandLine (" + cmd + ") may result in a security vulnerability if the path contains spaces."
134+
)
135+
select call, msg1 + " " + msg2

0 commit comments

Comments
 (0)