Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions cpp/ql/src/experimental/Security/CWE/CWE-078/WordexpTainted.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@

int main(int argc, char** argv) {
char *filePath = argv[2];

{
// BAD: the user-controlled string is injected
// directly into `wordexp` which performs command substitution

wordexp_t we;
wordexp(filePath, &we, 0);
}

{
// GOOD: command substitution is disabled

wordexp_t we;
wordexp(filePath, &we, WRDE_NOCMD);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>The code passes user input to <code>wordexp</code>. This leaves the code
vulnerable to attack by command injection, because <code>wordexp</code> performs command substitution.
Command substitution is a feature that replaces <code>$(command)</code> or <code>`command`</code> with the
output of the given command, allowing the user to run arbitrary code on the system.
</p>

</overview>
<recommendation>

<p>When calling <code>wordexp</code>, pass the <code>WRDE_NOCMD</code> flag to prevent command substitution.</p>

</recommendation>
<example>
<p>The following example passes a user-supplied file path to <code>wordexp</code> in two ways. The
first way uses <code>wordexp</code> with no specified flags. As such, it is vulnerable to command
injection.
The second way uses <code>wordexp</code> with the <code>WRDE_NOCMD</code> flag. As such, no command substitution
is performed, making this safe from command injection.</p>
<sample src="WordexpTainted.c" />

</example>
<references>

<li>CERT C Coding Standard:
<a href="https://www.securecoding.cert.org/confluence/display/c/STR02-C.+Sanitize+data+passed+to+complex+subsystems">STR02-C.
Sanitize data passed to complex subsystems</a>.</li>
<li>
OWASP:
<a href="https://www.owasp.org/index.php/Command_Injection">Command Injection</a>.
</li>


<!-- LocalWords: CWE STR
-->
Comment thread
intrigus-lgtm marked this conversation as resolved.

</references>
</qhelp>
57 changes: 57 additions & 0 deletions cpp/ql/src/experimental/Security/CWE/CWE-078/WordexpTainted.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/**
* @name Uncontrolled data used in `wordexp` command
* @description Using user-supplied data in a `wordexp` command, without
* disabling command substitution, can make code vulnerable
* to command injection.
* @kind path-problem
* @problem.severity error
* @precision high
* @id cpp/wordexp-injection
* @tags security
* external/cwe/cwe-078
*/

import cpp
import semmle.code.cpp.ir.dataflow.TaintTracking
import semmle.code.cpp.security.FlowSources
import DataFlow::PathGraph

/**
* The `wordexp` function, which can perform command substitution.
*/
private class WordexpFunction extends Function {
WordexpFunction() { hasGlobalName("wordexp") }
}

/**
* Holds if `fc` disables command substitution by containing `WRDE_NOCMD` as a flag argument.
*/
private predicate isCommandSubstitutionDisabled(FunctionCall fc) {
fc.getArgument(2).getValue().toInt().bitAnd(4) = 4
/* 4 = WRDE_NOCMD. Check whether the flag is set. */
}

/**
* A configuration to track user-supplied data to the `wordexp` function.
*/
class WordexpTaintConfiguration extends TaintTracking::Configuration {
WordexpTaintConfiguration() { this = "WordexpTaintConfiguration" }

override predicate isSource(DataFlow::Node source) { source instanceof FlowSource }

override predicate isSink(DataFlow::Node sink) {
exists(FunctionCall fc | fc.getTarget() instanceof WordexpFunction |
fc.getArgument(0) = sink.asExpr() and
not isCommandSubstitutionDisabled(fc)
)
}

override predicate isSanitizer(DataFlow::Node node) {
node.asExpr().getUnspecifiedType() instanceof IntegralType
}
}

from WordexpTaintConfiguration conf, DataFlow::PathNode sourceNode, DataFlow::PathNode sinkNode
where conf.hasFlowPath(sourceNode, sinkNode)
select sinkNode.getNode(), sourceNode, sinkNode,
"Using user-supplied data in a `wordexp` command, without disabling command substitution, can make code vulnerable to command injection."
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
edges
| test.cpp:23:20:23:23 | argv | test.cpp:29:13:29:20 | (const char *)... |
| test.cpp:23:20:23:23 | argv | test.cpp:29:13:29:20 | filePath |
nodes
| test.cpp:23:20:23:23 | argv | semmle.label | argv |
| test.cpp:29:13:29:20 | (const char *)... | semmle.label | (const char *)... |
| test.cpp:29:13:29:20 | filePath | semmle.label | filePath |
subpaths
#select
| test.cpp:29:13:29:20 | (const char *)... | test.cpp:23:20:23:23 | argv | test.cpp:29:13:29:20 | (const char *)... | Using user-supplied data in a `wordexp` command, without disabling command substitution, can make code vulnerable to command injection. |
Comment thread
intrigus-lgtm marked this conversation as resolved.
| test.cpp:29:13:29:20 | filePath | test.cpp:23:20:23:23 | argv | test.cpp:29:13:29:20 | filePath | Using user-supplied data in a `wordexp` command, without disabling command substitution, can make code vulnerable to command injection. |
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
experimental/Security/CWE/CWE-078/WordexpTainted.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
#ifdef _MSC_VER
#define restrict __restrict
#else
#define restrict __restrict__
#endif

typedef unsigned long size_t;

typedef struct {
size_t we_wordc;
char **we_wordv;
size_t we_offs;
} wordexp_t;

enum {
WRDE_APPEND = (1 << 1),
WRDE_NOCMD = (1 << 2)
};

int wordexp(const char *restrict s, wordexp_t *restrict p, int flags);

int main(int argc, char** argv) {
char *filePath = argv[2];

{
// BAD: the user string is injected directly into `wordexp` which performs command substitution

wordexp_t we;
wordexp(filePath, &we, 0);
}

{
// GOOD: command substitution is disabled

wordexp_t we;
wordexp(filePath, &we, WRDE_NOCMD);
}

{
// GOOD: command substitution is disabled

wordexp_t we;
wordexp(filePath, &we, WRDE_NOCMD | WRDE_APPEND);
}
}