-
Notifications
You must be signed in to change notification settings - Fork 2k
Add query for tainted wordexp calls.
#10077
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
geoffw0
merged 4 commits into
github:main
from
intrigus-lgtm:cpp/wexpand-commmand-injection
Oct 17, 2022
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
19 changes: 19 additions & 0 deletions
19
cpp/ql/src/experimental/Security/CWE/CWE-078/WordexpTainted.c
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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); | ||
| } | ||
| } |
42 changes: 42 additions & 0 deletions
42
cpp/ql/src/experimental/Security/CWE/CWE-078/WordexpTainted.qhelp
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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 | ||
| --> | ||
|
|
||
| </references> | ||
| </qhelp> | ||
57 changes: 57 additions & 0 deletions
57
cpp/ql/src/experimental/Security/CWE/CWE-078/WordexpTainted.ql
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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." |
11 changes: 11 additions & 0 deletions
11
cpp/ql/test/experimental/query-tests/Security/CWE/CWE-078/WordexpTainted.expected
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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. | | ||
|
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. | | ||
1 change: 1 addition & 0 deletions
1
cpp/ql/test/experimental/query-tests/Security/CWE/CWE-078/WordexpTainted.qlref
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| experimental/Security/CWE/CWE-078/WordexpTainted.ql |
45 changes: 45 additions & 0 deletions
45
cpp/ql/test/experimental/query-tests/Security/CWE/CWE-078/test.cpp
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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); | ||
| } | ||
| } |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.