diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-078/WordexpTainted.c b/cpp/ql/src/experimental/Security/CWE/CWE-078/WordexpTainted.c new file mode 100644 index 00000000000..63cd5488f44 --- /dev/null +++ b/cpp/ql/src/experimental/Security/CWE/CWE-078/WordexpTainted.c @@ -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); + } +} diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-078/WordexpTainted.qhelp b/cpp/ql/src/experimental/Security/CWE/CWE-078/WordexpTainted.qhelp new file mode 100644 index 00000000000..a9e4b6b98b2 --- /dev/null +++ b/cpp/ql/src/experimental/Security/CWE/CWE-078/WordexpTainted.qhelp @@ -0,0 +1,39 @@ + + + +

The code passes user input to wordexp. This leaves the code +vulnerable to attack by command injection, because wordexp performs command substitution.

+ +
+ + +

When calling wordexp, pass the WRDE_NOCMD flag to to prevent command substitution.

+ +
+ +

The following example passes a user-supplied file path to wordexp in two ways. The +first way uses wordexp with no specified flags. As such, it is vulnerable to command +injection. +The second way uses wordexp with the WRDE_NOCMD flag. As such, no command substitution +is performed, making this safe from command injection.

+ + +
+ + +
  • CERT C Coding Standard: +STR02-C. +Sanitize data passed to complex subsystems.
  • +
  • +OWASP: +Command Injection. +
  • + + + + +
    +
    diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-078/WordexpTainted.ql b/cpp/ql/src/experimental/Security/CWE/CWE-078/WordexpTainted.ql new file mode 100644 index 00000000000..43922b79110 --- /dev/null +++ b/cpp/ql/src/experimental/Security/CWE/CWE-078/WordexpTainted.ql @@ -0,0 +1,53 @@ +/** + * @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) + ) + } +} + +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." diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-078/WordexpTainted.expected b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-078/WordexpTainted.expected new file mode 100644 index 00000000000..a8d7a480c81 --- /dev/null +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-078/WordexpTainted.expected @@ -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. | +| 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. | diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-078/WordexpTainted.qlref b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-078/WordexpTainted.qlref new file mode 100644 index 00000000000..ecff539f3e6 --- /dev/null +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-078/WordexpTainted.qlref @@ -0,0 +1 @@ +experimental/Security/CWE/CWE-078/WordexpTainted.ql \ No newline at end of file diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-078/test.cpp b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-078/test.cpp new file mode 100644 index 00000000000..0ae98b8f163 --- /dev/null +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-078/test.cpp @@ -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); + } +}