hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-05-03 12:45:27 +02:00
Code Issues Packages Projects Releases Wiki Activity

JS: Restrict getInput to indirect command injection query

Browse Source
This commit is contained in:
Asger F
2023-05-03 16:10:03 +02:00
parent b9ad4177f9
commit 1a9956354e
6 changed files with 43 additions and 64 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
javascript/ql/lib/semmle/javascript/frameworks/ActionsLib.qll
View File

@@ -3,6 +3,7 @@
*/
private import javascript
private import semmle.javascript.security.dataflow.IndirectCommandInjectionCustomizations
private API::Node payload() {
result = API::moduleImport("@actions/github").getMember("context").getMember("payload")
@@ -51,11 +52,10 @@ private class GitHubActionsContextSource extends RemoteFlowSource {
/**
* A source of taint originating from user input.
*
* At the momemnt this is treated as a remote flow source, although it is not
* always possible for an attacker to control this. In the future we might classify
* this differently.
* At the momemnt this is only treated as a taint source for the indirect-command injection
* query.
*/
private class GitHubActionsInputSource extends RemoteFlowSource {
private class GitHubActionsInputSource extends IndirectCommandInjection::Source {
GitHubActionsInputSource() {
this =
API::moduleImport("@actions/core")
@@ -64,7 +64,7 @@ private class GitHubActionsInputSource extends RemoteFlowSource {
.asSource()
}
override string getSourceType() { result = "GitHub Actions user input" }
override string describe() { result = "GitHub Actions user input" }
}
private class ExecActionsCall extends SystemCommandExecution, DataFlow::CallNode {

47
javascript/ql/test/query-tests/Security/CWE-078/IndirectCommandInjection/IndirectCommandInjection.expected
View File

@@ -1,14 +1,17 @@
nodes
| actions.js:3:6:3:16 | process.env |
| actions.js:3:6:3:16 | process.env |
| actions.js:3:6:3:29 | process ... _DATA'] |
| actions.js:3:6:3:29 | process ... _DATA'] |
| actions.js:6:15:6:15 | e |
| actions.js:7:10:7:10 | e |
| actions.js:7:10:7:23 | e['TEST_DATA'] |
| actions.js:7:10:7:23 | e['TEST_DATA'] |
| actions.js:11:6:11:16 | process.env |
| actions.js:11:6:11:16 | process.env |
| actions.js:4:6:4:16 | process.env |
| actions.js:4:6:4:16 | process.env |
| actions.js:4:6:4:29 | process ... _DATA'] |
| actions.js:4:6:4:29 | process ... _DATA'] |
| actions.js:7:15:7:15 | e |
| actions.js:8:10:8:10 | e |
| actions.js:8:10:8:23 | e['TEST_DATA'] |
| actions.js:8:10:8:23 | e['TEST_DATA'] |
| actions.js:12:6:12:16 | process.env |
| actions.js:12:6:12:16 | process.env |
| actions.js:14:6:14:21 | getInput('data') |
| actions.js:14:6:14:21 | getInput('data') |
| actions.js:14:6:14:21 | getInput('data') |
| command-line-parameter-command-injection.js:4:10:4:21 | process.argv |
| command-line-parameter-command-injection.js:4:10:4:21 | process.argv |
| command-line-parameter-command-injection.js:4:10:4:21 | process.argv |
@@ -222,15 +225,16 @@ nodes
| command-line-parameter-command-injection.js:146:22:146:38 | program.pizzaType |
| command-line-parameter-command-injection.js:146:22:146:38 | program.pizzaType |
edges
| actions.js:3:6:3:16 | process.env | actions.js:3:6:3:29 | process ... _DATA'] |
| actions.js:3:6:3:16 | process.env | actions.js:3:6:3:29 | process ... _DATA'] |
| actions.js:3:6:3:16 | process.env | actions.js:3:6:3:29 | process ... _DATA'] |
| actions.js:3:6:3:16 | process.env | actions.js:3:6:3:29 | process ... _DATA'] |
| actions.js:6:15:6:15 | e | actions.js:7:10:7:10 | e |
| actions.js:7:10:7:10 | e | actions.js:7:10:7:23 | e['TEST_DATA'] |
| actions.js:7:10:7:10 | e | actions.js:7:10:7:23 | e['TEST_DATA'] |
| actions.js:11:6:11:16 | process.env | actions.js:6:15:6:15 | e |
| actions.js:11:6:11:16 | process.env | actions.js:6:15:6:15 | e |
| actions.js:4:6:4:16 | process.env | actions.js:4:6:4:29 | process ... _DATA'] |
| actions.js:4:6:4:16 | process.env | actions.js:4:6:4:29 | process ... _DATA'] |
| actions.js:4:6:4:16 | process.env | actions.js:4:6:4:29 | process ... _DATA'] |
| actions.js:4:6:4:16 | process.env | actions.js:4:6:4:29 | process ... _DATA'] |
| actions.js:7:15:7:15 | e | actions.js:8:10:8:10 | e |
| actions.js:8:10:8:10 | e | actions.js:8:10:8:23 | e['TEST_DATA'] |
| actions.js:8:10:8:10 | e | actions.js:8:10:8:23 | e['TEST_DATA'] |
| actions.js:12:6:12:16 | process.env | actions.js:7:15:7:15 | e |
| actions.js:12:6:12:16 | process.env | actions.js:7:15:7:15 | e |
| actions.js:14:6:14:21 | getInput('data') | actions.js:14:6:14:21 | getInput('data') |
| command-line-parameter-command-injection.js:4:10:4:21 | process.argv | command-line-parameter-command-injection.js:4:10:4:21 | process.argv |
| command-line-parameter-command-injection.js:8:22:8:33 | process.argv | command-line-parameter-command-injection.js:8:22:8:36 | process.argv[2] |
| command-line-parameter-command-injection.js:8:22:8:33 | process.argv | command-line-parameter-command-injection.js:8:22:8:36 | process.argv[2] |
@@ -419,8 +423,9 @@ edges
| command-line-parameter-command-injection.js:146:22:146:38 | program.pizzaType | command-line-parameter-command-injection.js:146:10:146:38 | "cmd.sh ... zzaType |
| command-line-parameter-command-injection.js:146:22:146:38 | program.pizzaType | command-line-parameter-command-injection.js:146:10:146:38 | "cmd.sh ... zzaType |
#select
| actions.js:3:6:3:29 | process ... _DATA'] | actions.js:3:6:3:16 | process.env | actions.js:3:6:3:29 | process ... _DATA'] | This command depends on an unsanitized $@. | actions.js:3:6:3:16 | process.env | environment variable |
| actions.js:7:10:7:23 | e['TEST_DATA'] | actions.js:11:6:11:16 | process.env | actions.js:7:10:7:23 | e['TEST_DATA'] | This command depends on an unsanitized $@. | actions.js:11:6:11:16 | process.env | environment variable |
| actions.js:4:6:4:29 | process ... _DATA'] | actions.js:4:6:4:16 | process.env | actions.js:4:6:4:29 | process ... _DATA'] | This command depends on an unsanitized $@. | actions.js:4:6:4:16 | process.env | environment variable |
| actions.js:8:10:8:23 | e['TEST_DATA'] | actions.js:12:6:12:16 | process.env | actions.js:8:10:8:23 | e['TEST_DATA'] | This command depends on an unsanitized $@. | actions.js:12:6:12:16 | process.env | environment variable |
| actions.js:14:6:14:21 | getInput('data') | actions.js:14:6:14:21 | getInput('data') | actions.js:14:6:14:21 | getInput('data') | This command depends on an unsanitized $@. | actions.js:14:6:14:21 | getInput('data') | GitHub Actions user input |
| command-line-parameter-command-injection.js:4:10:4:21 | process.argv | command-line-parameter-command-injection.js:4:10:4:21 | process.argv | command-line-parameter-command-injection.js:4:10:4:21 | process.argv | This command depends on an unsanitized $@. | command-line-parameter-command-injection.js:4:10:4:21 | process.argv | command-line argument |
| command-line-parameter-command-injection.js:8:10:8:36 | "cmd.sh ... argv[2] | command-line-parameter-command-injection.js:8:22:8:33 | process.argv | command-line-parameter-command-injection.js:8:10:8:36 | "cmd.sh ... argv[2] | This command depends on an unsanitized $@. | command-line-parameter-command-injection.js:8:22:8:33 | process.argv | command-line argument |
| command-line-parameter-command-injection.js:11:14:11:20 | args[0] | command-line-parameter-command-injection.js:10:13:10:24 | process.argv | command-line-parameter-command-injection.js:11:14:11:20 | args[0] | This command depends on an unsanitized $@. | command-line-parameter-command-injection.js:10:13:10:24 | process.argv | command-line argument |

3
javascript/ql/test/query-tests/Security/CWE-078/IndirectCommandInjection/actions.js
View File

@@ -1,4 +1,5 @@
import { exec } from "@actions/exec";
import { getInput } from "@actions/core";
exec(process.env['TEST_DATA']); // NOT OK
exec(process.env['GITHUB_ACTION']); // OK
@@ -9,3 +10,5 @@ function test(e) {
}
test(process.env);
exec(getInput('data')); // NOT OK

24
javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/CodeInjection.expected
View File

@@ -13,16 +13,9 @@ nodes
| NoSQLCodeInjection.js:22:36:22:43 | req.body |
| NoSQLCodeInjection.js:22:36:22:43 | req.body |
| NoSQLCodeInjection.js:22:36:22:48 | req.body.name |
| actions.js:5:10:5:50 | github. ... message |
| actions.js:5:10:5:50 | github. ... message |
| actions.js:5:10:5:50 | github. ... message |
| actions.js:6:10:6:33 | core.ge ... mbers') |
| actions.js:6:10:6:33 | core.ge ... mbers') |
| actions.js:6:10:6:33 | core.ge ... mbers') |
| actions.js:7:10:7:42 | core.ge ... mbers') |
| actions.js:7:10:7:42 | core.ge ... mbers') |
| actions.js:7:10:7:53 | core.ge ... n('\\n') |
| actions.js:7:10:7:53 | core.ge ... n('\\n') |
| actions.js:4:10:4:50 | github. ... message |
| actions.js:4:10:4:50 | github. ... message |
| actions.js:4:10:4:50 | github. ... message |
| angularjs.js:10:22:10:36 | location.search |
| angularjs.js:10:22:10:36 | location.search |
| angularjs.js:10:22:10:36 | location.search |
@@ -201,12 +194,7 @@ edges
| NoSQLCodeInjection.js:22:36:22:43 | req.body | NoSQLCodeInjection.js:22:36:22:48 | req.body.name |
| NoSQLCodeInjection.js:22:36:22:48 | req.body.name | NoSQLCodeInjection.js:22:24:22:48 | "name = ... dy.name |
| NoSQLCodeInjection.js:22:36:22:48 | req.body.name | NoSQLCodeInjection.js:22:24:22:48 | "name = ... dy.name |
| actions.js:5:10:5:50 | github. ... message | actions.js:5:10:5:50 | github. ... message |
| actions.js:6:10:6:33 | core.ge ... mbers') | actions.js:6:10:6:33 | core.ge ... mbers') |
| actions.js:7:10:7:42 | core.ge ... mbers') | actions.js:7:10:7:53 | core.ge ... n('\\n') |
| actions.js:7:10:7:42 | core.ge ... mbers') | actions.js:7:10:7:53 | core.ge ... n('\\n') |
| actions.js:7:10:7:42 | core.ge ... mbers') | actions.js:7:10:7:53 | core.ge ... n('\\n') |
| actions.js:7:10:7:42 | core.ge ... mbers') | actions.js:7:10:7:53 | core.ge ... n('\\n') |
| actions.js:4:10:4:50 | github. ... message | actions.js:4:10:4:50 | github. ... message |
| angularjs.js:10:22:10:36 | location.search | angularjs.js:10:22:10:36 | location.search |
| angularjs.js:13:23:13:37 | location.search | angularjs.js:13:23:13:37 | location.search |
| angularjs.js:16:28:16:42 | location.search | angularjs.js:16:28:16:42 | location.search |
@@ -322,9 +310,7 @@ edges
| NoSQLCodeInjection.js:18:24:18:37 | req.body.query | NoSQLCodeInjection.js:18:24:18:31 | req.body | NoSQLCodeInjection.js:18:24:18:37 | req.body.query | This code execution depends on a $@. | NoSQLCodeInjection.js:18:24:18:31 | req.body | user-provided value |
| NoSQLCodeInjection.js:19:24:19:48 | "name = ... dy.name | NoSQLCodeInjection.js:19:36:19:43 | req.body | NoSQLCodeInjection.js:19:24:19:48 | "name = ... dy.name | This code execution depends on a $@. | NoSQLCodeInjection.js:19:36:19:43 | req.body | user-provided value |
| NoSQLCodeInjection.js:22:24:22:48 | "name = ... dy.name | NoSQLCodeInjection.js:22:36:22:43 | req.body | NoSQLCodeInjection.js:22:24:22:48 | "name = ... dy.name | This code execution depends on a $@. | NoSQLCodeInjection.js:22:36:22:43 | req.body | user-provided value |
| actions.js:5:10:5:50 | github. ... message | actions.js:5:10:5:50 | github. ... message | actions.js:5:10:5:50 | github. ... message | This code execution depends on a $@. | actions.js:5:10:5:50 | github. ... message | user-provided value |
| actions.js:6:10:6:33 | core.ge ... mbers') | actions.js:6:10:6:33 | core.ge ... mbers') | actions.js:6:10:6:33 | core.ge ... mbers') | This code execution depends on a $@. | actions.js:6:10:6:33 | core.ge ... mbers') | user-provided value |
| actions.js:7:10:7:53 | core.ge ... n('\\n') | actions.js:7:10:7:42 | core.ge ... mbers') | actions.js:7:10:7:53 | core.ge ... n('\\n') | This code execution depends on a $@. | actions.js:7:10:7:42 | core.ge ... mbers') | user-provided value |
| actions.js:4:10:4:50 | github. ... message | actions.js:4:10:4:50 | github. ... message | actions.js:4:10:4:50 | github. ... message | This code execution depends on a $@. | actions.js:4:10:4:50 | github. ... message | user-provided value |
| angularjs.js:10:22:10:36 | location.search | angularjs.js:10:22:10:36 | location.search | angularjs.js:10:22:10:36 | location.search | This code execution depends on a $@. | angularjs.js:10:22:10:36 | location.search | user-provided value |
| angularjs.js:13:23:13:37 | location.search | angularjs.js:13:23:13:37 | location.search | angularjs.js:13:23:13:37 | location.search | This code execution depends on a $@. | angularjs.js:13:23:13:37 | location.search | user-provided value |
| angularjs.js:16:28:16:42 | location.search | angularjs.js:16:28:16:42 | location.search | angularjs.js:16:28:16:42 | location.search | This code execution depends on a $@. | angularjs.js:16:28:16:42 | location.search | user-provided value |

20
javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/HeuristicSourceCodeInjection.expected
View File

@@ -13,16 +13,9 @@ nodes
| NoSQLCodeInjection.js:22:36:22:43 | req.body |
| NoSQLCodeInjection.js:22:36:22:43 | req.body |
| NoSQLCodeInjection.js:22:36:22:48 | req.body.name |
| actions.js:5:10:5:50 | github. ... message |
| actions.js:5:10:5:50 | github. ... message |
| actions.js:5:10:5:50 | github. ... message |
| actions.js:6:10:6:33 | core.ge ... mbers') |
| actions.js:6:10:6:33 | core.ge ... mbers') |
| actions.js:6:10:6:33 | core.ge ... mbers') |
| actions.js:7:10:7:42 | core.ge ... mbers') |
| actions.js:7:10:7:42 | core.ge ... mbers') |
| actions.js:7:10:7:53 | core.ge ... n('\\n') |
| actions.js:7:10:7:53 | core.ge ... n('\\n') |
| actions.js:4:10:4:50 | github. ... message |
| actions.js:4:10:4:50 | github. ... message |
| actions.js:4:10:4:50 | github. ... message |
| angularjs.js:10:22:10:36 | location.search |
| angularjs.js:10:22:10:36 | location.search |
| angularjs.js:10:22:10:36 | location.search |
@@ -205,12 +198,7 @@ edges
| NoSQLCodeInjection.js:22:36:22:43 | req.body | NoSQLCodeInjection.js:22:36:22:48 | req.body.name |
| NoSQLCodeInjection.js:22:36:22:48 | req.body.name | NoSQLCodeInjection.js:22:24:22:48 | "name = ... dy.name |
| NoSQLCodeInjection.js:22:36:22:48 | req.body.name | NoSQLCodeInjection.js:22:24:22:48 | "name = ... dy.name |
| actions.js:5:10:5:50 | github. ... message | actions.js:5:10:5:50 | github. ... message |
| actions.js:6:10:6:33 | core.ge ... mbers') | actions.js:6:10:6:33 | core.ge ... mbers') |
| actions.js:7:10:7:42 | core.ge ... mbers') | actions.js:7:10:7:53 | core.ge ... n('\\n') |
| actions.js:7:10:7:42 | core.ge ... mbers') | actions.js:7:10:7:53 | core.ge ... n('\\n') |
| actions.js:7:10:7:42 | core.ge ... mbers') | actions.js:7:10:7:53 | core.ge ... n('\\n') |
| actions.js:7:10:7:42 | core.ge ... mbers') | actions.js:7:10:7:53 | core.ge ... n('\\n') |
| actions.js:4:10:4:50 | github. ... message | actions.js:4:10:4:50 | github. ... message |
| angularjs.js:10:22:10:36 | location.search | angularjs.js:10:22:10:36 | location.search |
| angularjs.js:13:23:13:37 | location.search | angularjs.js:13:23:13:37 | location.search |
| angularjs.js:16:28:16:42 | location.search | angularjs.js:16:28:16:42 | location.search |

3
javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/actions.js
View File

@@ -1,8 +1,5 @@
const core = require('@actions/core');
const github = require('@actions/github');
function test() {
eval(github.context.payload.commits[1].message); // NOT OK
eval(core.getInput('numbers')); // NOT OK
eval(core.getMultilineInput('numbers').join('\n')); // NOT OK
}