Add new tests

Alvaro Muñoz · Alvaro Muñoz · commit 46afa9c1f3fc · 2024-03-14T22:41:01.000+01:00
diff --git a/ql/lib/codeql/actions/dataflow/ExternalFlow.qll b/ql/lib/codeql/actions/dataflow/ExternalFlow.qll
@@ -86,8 +86,10 @@ predicate externallyDefinedStoreStep(
   )
 }
 
-predicate externallyDefinedSink(DataFlow::ExprNode sink, string kind) {
+predicate externallyDefinedSink(DataFlow::Node sink, string kind) {
   exists(Uses uses, string action, string version, string input |
+    sinkModel(action, version, input, kind) and
+    uses.getCallee() = action.toLowerCase() and
     (
       if input.trim().matches("env.%")
       then sink.asExpr() = uses.getInScopeEnvVarExpr(input.trim().replaceAll("env.", ""))
@@ -96,8 +98,6 @@ predicate externallyDefinedSink(DataFlow::ExprNode sink, string kind) {
         then sink.asExpr() = uses.getArgumentExpr(input.trim().replaceAll("input.", ""))
         else none()
     ) and
-    sinkModel(action, version, input, kind) and
-    uses.getCallee() = action.toLowerCase() and
     (
       if version.trim() = "*"
       then uses.getVersion() = any(string v)
diff --git a/ql/src/Security/CWE-078/CommandInjection.ql b/ql/src/Security/CWE-078/CommandInjection.ql
@@ -13,6 +13,7 @@
  */
 
 import actions
+import codeql.actions.DataFlow
 import codeql.actions.TaintTracking
 import codeql.actions.dataflow.FlowSources
 import codeql.actions.dataflow.ExternalFlow
diff --git a/ql/src/Security/CWE-078/CriticalCommandInjection.ql b/ql/src/Security/CWE-078/CriticalCommandInjection.ql
@@ -13,6 +13,7 @@
  */
 
 import actions
+import codeql.actions.DataFlow
 import codeql.actions.TaintTracking
 import codeql.actions.dataflow.FlowSources
 import codeql.actions.dataflow.ExternalFlow
diff --git a/ql/src/Security/CWE-094/CodeInjection.ql b/ql/src/Security/CWE-094/CodeInjection.ql
@@ -15,6 +15,7 @@
  */
 
 import actions
+import codeql.actions.DataFlow
 import codeql.actions.TaintTracking
 import codeql.actions.dataflow.FlowSources
 import codeql.actions.dataflow.ExternalFlow
diff --git a/ql/src/Security/CWE-094/CriticalCodeInjection.ql b/ql/src/Security/CWE-094/CriticalCodeInjection.ql
@@ -15,6 +15,7 @@
  */
 
 import actions
+import codeql.actions.DataFlow
 import codeql.actions.TaintTracking
 import codeql.actions.dataflow.FlowSources
 import codeql.actions.dataflow.ExternalFlow
diff --git a/ql/src/Security/CWE-918/RequestForgery.ql b/ql/src/Security/CWE-918/RequestForgery.ql
@@ -12,6 +12,7 @@
  */
 
 import actions
+import codeql.actions.DataFlow
 import codeql.actions.TaintTracking
 import codeql.actions.dataflow.FlowSources
 import codeql.actions.dataflow.ExternalFlow
diff --git a/ql/test/library-tests/test.expected b/ql/test/library-tests/test.expected
@@ -315,6 +315,9 @@ scopes
 sources
 | ahmadnassri/action-changed-files | * | output.files | pull_request_target | PR changed files |
 | ahmadnassri/action-changed-files | * | output.json | pull_request_target | PR changed files |
+| amannn/action-semantic-pull-request | * | output.error_message | pull_request_target | PR title |
+| cypress-io/github-action | * | env.GH_BRANCH | pull_request_target | PR branch |
+| dawidd6/action-download-artifact | * | output.artifacts | * | Artifact details |
 | dorny/paths-filter | * | output.changes | pull_request_target | PR changed files |
 | franzdiebold/github-env-vars-action | * | output.CI_PR_DESCRIPTION | pull_request_target | PR body |
 | franzdiebold/github-env-vars-action | * | output.CI_PR_TITLE | pull_request_target | PR title |
diff --git a/ql/test/query-tests/Security/CWE-078/.github/workflows/comment_issue.yml b/ql/test/query-tests/Security/CWE-078/.github/workflows/comment_issue.yml
@@ -0,0 +1,9 @@
+on: issue_comment
+
+jobs:
+  test1:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: ruby/setup-ruby@v2
+        with:
+          ruby-version: ${{ github.event.comment.body }}
diff --git a/ql/test/query-tests/Security/CWE-078/CommandInjection.expected b/ql/test/query-tests/Security/CWE-078/CommandInjection.expected
@@ -0,0 +1,6 @@
+edges
+nodes
+| .github/workflows/comment_issue.yml:9:26:9:57 | github.event.comment.body | semmle.label | github.event.comment.body |
+subpaths
+#select
+| .github/workflows/comment_issue.yml:9:26:9:57 | github.event.comment.body | .github/workflows/comment_issue.yml:9:26:9:57 | github.event.comment.body | .github/workflows/comment_issue.yml:9:26:9:57 | github.event.comment.body | Potential command injection in $@, which may be controlled by an external user. | .github/workflows/comment_issue.yml:9:26:9:57 | github.event.comment.body | ${{ github.event.comment.body }} |
diff --git a/ql/test/query-tests/Security/CWE-078/CommandInjection.qlref b/ql/test/query-tests/Security/CWE-078/CommandInjection.qlref
@@ -0,0 +1 @@
+Security/CWE-078/CommandInjection.ql
diff --git a/ql/test/query-tests/Security/CWE-078/CriticalCommandInjection.expected b/ql/test/query-tests/Security/CWE-078/CriticalCommandInjection.expected
@@ -0,0 +1,6 @@
+edges
+nodes
+| .github/workflows/comment_issue.yml:9:26:9:57 | github.event.comment.body | semmle.label | github.event.comment.body |
+subpaths
+#select
+| .github/workflows/comment_issue.yml:9:26:9:57 | github.event.comment.body | .github/workflows/comment_issue.yml:9:26:9:57 | github.event.comment.body | .github/workflows/comment_issue.yml:9:26:9:57 | github.event.comment.body | Potential critical command injection in $@, which may be controlled by an external user. | .github/workflows/comment_issue.yml:9:26:9:57 | github.event.comment.body | ${{ github.event.comment.body }} |
diff --git a/ql/test/query-tests/Security/CWE-078/CriticalCommandInjection.qlref b/ql/test/query-tests/Security/CWE-078/CriticalCommandInjection.qlref
@@ -0,0 +1 @@
+Security/CWE-078/CriticalCommandInjection.ql
diff --git a/ql/test/query-tests/Security/CWE-094/CodeInjection.expected b/ql/test/query-tests/Security/CWE-094/CodeInjection.expected
@@ -0,0 +1,14 @@
+edges
+| .github/workflows/changelog.yml:49:19:49:56 | github.event.pull_request.title | .github/workflows/changelog.yml:58:26:58:39 | env.log |
+nodes
+| .github/workflows/changelog.yml:49:19:49:56 | github.event.pull_request.title | semmle.label | github.event.pull_request.title |
+| .github/workflows/changelog.yml:58:26:58:39 | env.log | semmle.label | env.log |
+| .github/workflows/comment_issue.yml:24:31:24:62 | github.event.comment.body | semmle.label | github.event.comment.body |
+| .github/workflows/comment_issue.yml:27:31:27:60 | github.event.issue.body | semmle.label | github.event.issue.body |
+| .github/workflows/comment_issue.yml:30:31:30:61 | github.event.issue.title | semmle.label | github.event.issue.title |
+subpaths
+#select
+| .github/workflows/changelog.yml:58:26:58:39 | env.log | .github/workflows/changelog.yml:49:19:49:56 | github.event.pull_request.title | .github/workflows/changelog.yml:58:26:58:39 | env.log | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/changelog.yml:58:26:58:39 | env.log | ${{ env.log }} |
+| .github/workflows/comment_issue.yml:24:31:24:62 | github.event.comment.body | .github/workflows/comment_issue.yml:24:31:24:62 | github.event.comment.body | .github/workflows/comment_issue.yml:24:31:24:62 | github.event.comment.body | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/comment_issue.yml:24:31:24:62 | github.event.comment.body | ${{ github.event.comment.body }} |
+| .github/workflows/comment_issue.yml:27:31:27:60 | github.event.issue.body | .github/workflows/comment_issue.yml:27:31:27:60 | github.event.issue.body | .github/workflows/comment_issue.yml:27:31:27:60 | github.event.issue.body | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/comment_issue.yml:27:31:27:60 | github.event.issue.body | ${{ github.event.issue.body }} |
+| .github/workflows/comment_issue.yml:30:31:30:61 | github.event.issue.title | .github/workflows/comment_issue.yml:30:31:30:61 | github.event.issue.title | .github/workflows/comment_issue.yml:30:31:30:61 | github.event.issue.title | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/comment_issue.yml:30:31:30:61 | github.event.issue.title | ${{ github.event.issue.title }} |
diff --git a/ql/test/query-tests/Security/CWE-094/CodeInjection.qlref b/ql/test/query-tests/Security/CWE-094/CodeInjection.qlref
@@ -0,0 +1 @@
+Security/CWE-094/CodeInjection.ql
diff --git a/ql/test/query-tests/Security/CWE-094/CriticalCodeInjection.expected b/ql/test/query-tests/Security/CWE-094/CriticalCodeInjection.expected
@@ -0,0 +1,14 @@
+edges
+| .github/workflows/changelog.yml:49:19:49:56 | github.event.pull_request.title | .github/workflows/changelog.yml:58:26:58:39 | env.log |
+nodes
+| .github/workflows/changelog.yml:49:19:49:56 | github.event.pull_request.title | semmle.label | github.event.pull_request.title |
+| .github/workflows/changelog.yml:58:26:58:39 | env.log | semmle.label | env.log |
+| .github/workflows/comment_issue.yml:24:31:24:62 | github.event.comment.body | semmle.label | github.event.comment.body |
+| .github/workflows/comment_issue.yml:27:31:27:60 | github.event.issue.body | semmle.label | github.event.issue.body |
+| .github/workflows/comment_issue.yml:30:31:30:61 | github.event.issue.title | semmle.label | github.event.issue.title |
+subpaths
+#select
+| .github/workflows/changelog.yml:58:26:58:39 | env.log | .github/workflows/changelog.yml:49:19:49:56 | github.event.pull_request.title | .github/workflows/changelog.yml:58:26:58:39 | env.log | Potential critical code injection in $@, which may be controlled by an external user. | .github/workflows/changelog.yml:58:26:58:39 | env.log | ${{ env.log }} |
+| .github/workflows/comment_issue.yml:24:31:24:62 | github.event.comment.body | .github/workflows/comment_issue.yml:24:31:24:62 | github.event.comment.body | .github/workflows/comment_issue.yml:24:31:24:62 | github.event.comment.body | Potential critical code injection in $@, which may be controlled by an external user. | .github/workflows/comment_issue.yml:24:31:24:62 | github.event.comment.body | ${{ github.event.comment.body }} |
+| .github/workflows/comment_issue.yml:27:31:27:60 | github.event.issue.body | .github/workflows/comment_issue.yml:27:31:27:60 | github.event.issue.body | .github/workflows/comment_issue.yml:27:31:27:60 | github.event.issue.body | Potential critical code injection in $@, which may be controlled by an external user. | .github/workflows/comment_issue.yml:27:31:27:60 | github.event.issue.body | ${{ github.event.issue.body }} |
+| .github/workflows/comment_issue.yml:30:31:30:61 | github.event.issue.title | .github/workflows/comment_issue.yml:30:31:30:61 | github.event.issue.title | .github/workflows/comment_issue.yml:30:31:30:61 | github.event.issue.title | Potential critical code injection in $@, which may be controlled by an external user. | .github/workflows/comment_issue.yml:30:31:30:61 | github.event.issue.title | ${{ github.event.issue.title }} |
diff --git a/ql/test/query-tests/Security/CWE-094/CriticalCodeInjection.qlref b/ql/test/query-tests/Security/CWE-094/CriticalCodeInjection.qlref
@@ -0,0 +1 @@
+Security/CWE-094/CriticalCodeInjection.ql
diff --git a/ql/test/query-tests/Security/CWE-094/CriticalExpressionInjection.expected b/ql/test/query-tests/Security/CWE-094/CriticalExpressionInjection.expected
@@ -3,7 +3,6 @@ edges
 | .github/workflows/argus_case_study.yml:17:25:17:53 | github.event.issue.title | .github/workflows/argus_case_study.yml:22:20:22:39 | env.ISSUE_TITLE |
 | .github/workflows/argus_case_study.yml:22:20:22:39 | env.ISSUE_TITLE | .github/workflows/argus_case_study.yml:15:9:24:6 | Uses Step: remove_quotations [replaced] |
 | .github/workflows/changed-files.yml:16:9:20:6 | Uses Step: changed-files | .github/workflows/changed-files.yml:22:24:22:75 | steps.changed-files.outputs.all_changed_files |
-| .github/workflows/changelog.yml:49:19:49:56 | github.event.pull_request.title | .github/workflows/changelog.yml:58:26:58:39 | env.log |
 | .github/workflows/cross3.yml:27:7:37:4 | Uses Step: remove_quotations [replaced] | .github/workflows/cross3.yml:39:31:39:75 | steps.remove_quotations.outputs.replaced |
 | .github/workflows/cross3.yml:27:7:37:4 | Uses Step: remove_quotations [replaced] | .github/workflows/cross3.yml:57:29:57:73 | steps.remove_quotations.outputs.replaced |
 | .github/workflows/cross3.yml:32:18:32:53 | github.event.commits[0].message | .github/workflows/cross3.yml:27:7:37:4 | Uses Step: remove_quotations [replaced] |
@@ -64,15 +63,10 @@ nodes
 | .github/workflows/argus_case_study.yml:27:33:27:77 | steps.remove_quotations.outputs.replaced | semmle.label | steps.remove_quotations.outputs.replaced |
 | .github/workflows/changed-files.yml:16:9:20:6 | Uses Step: changed-files | semmle.label | Uses Step: changed-files |
 | .github/workflows/changed-files.yml:22:24:22:75 | steps.changed-files.outputs.all_changed_files | semmle.label | steps.changed-files.outputs.all_changed_files |
-| .github/workflows/changelog.yml:49:19:49:56 | github.event.pull_request.title | semmle.label | github.event.pull_request.title |
-| .github/workflows/changelog.yml:58:26:58:39 | env.log | semmle.label | env.log |
 | .github/workflows/comment_issue.yml:9:15:9:46 | github.event.comment.body | semmle.label | github.event.comment.body |
 | .github/workflows/comment_issue.yml:15:19:15:50 | github.event.comment.body | semmle.label | github.event.comment.body |
 | .github/workflows/comment_issue.yml:16:19:16:48 | github.event.issue.body | semmle.label | github.event.issue.body |
 | .github/workflows/comment_issue.yml:17:19:17:49 | github.event.issue.title | semmle.label | github.event.issue.title |
-| .github/workflows/comment_issue.yml:24:31:24:62 | github.event.comment.body | semmle.label | github.event.comment.body |
-| .github/workflows/comment_issue.yml:27:31:27:60 | github.event.issue.body | semmle.label | github.event.issue.body |
-| .github/workflows/comment_issue.yml:30:31:30:61 | github.event.issue.title | semmle.label | github.event.issue.title |
 | .github/workflows/comment_issue_newline.yml:10:25:10:56 | github.event.comment.body | semmle.label | github.event.comment.body |
 | .github/workflows/comment_issue_newline.yml:11:24:11:51 | github.event.issue.body | semmle.label | github.event.issue.body |
 | .github/workflows/comment_issue_newline.yml:12:24:12:55 | github.event.comment.body | semmle.label | github.event.comment.body |
@@ -201,14 +195,10 @@ nodes
 subpaths
 #select
 | .github/workflows/argus_case_study.yml:27:33:27:77 | steps.remove_quotations.outputs.replaced | .github/workflows/argus_case_study.yml:17:25:17:53 | github.event.issue.title | .github/workflows/argus_case_study.yml:27:33:27:77 | steps.remove_quotations.outputs.replaced | Potential expression injection in $@, which may be controlled by an external user. | .github/workflows/argus_case_study.yml:27:33:27:77 | steps.remove_quotations.outputs.replaced | steps.remove_quotations.outputs.replaced |
-| .github/workflows/changelog.yml:58:26:58:39 | env.log | .github/workflows/changelog.yml:49:19:49:56 | github.event.pull_request.title | .github/workflows/changelog.yml:58:26:58:39 | env.log | Potential expression injection in $@, which may be controlled by an external user. | .github/workflows/changelog.yml:58:26:58:39 | env.log | env.log |
 | .github/workflows/comment_issue.yml:9:15:9:46 | github.event.comment.body | .github/workflows/comment_issue.yml:9:15:9:46 | github.event.comment.body | .github/workflows/comment_issue.yml:9:15:9:46 | github.event.comment.body | Potential expression injection in $@, which may be controlled by an external user. | .github/workflows/comment_issue.yml:9:15:9:46 | github.event.comment.body | github.event.comment.body |
 | .github/workflows/comment_issue.yml:15:19:15:50 | github.event.comment.body | .github/workflows/comment_issue.yml:15:19:15:50 | github.event.comment.body | .github/workflows/comment_issue.yml:15:19:15:50 | github.event.comment.body | Potential expression injection in $@, which may be controlled by an external user. | .github/workflows/comment_issue.yml:15:19:15:50 | github.event.comment.body | github.event.comment.body |
 | .github/workflows/comment_issue.yml:16:19:16:48 | github.event.issue.body | .github/workflows/comment_issue.yml:16:19:16:48 | github.event.issue.body | .github/workflows/comment_issue.yml:16:19:16:48 | github.event.issue.body | Potential expression injection in $@, which may be controlled by an external user. | .github/workflows/comment_issue.yml:16:19:16:48 | github.event.issue.body | github.event.issue.body |
 | .github/workflows/comment_issue.yml:17:19:17:49 | github.event.issue.title | .github/workflows/comment_issue.yml:17:19:17:49 | github.event.issue.title | .github/workflows/comment_issue.yml:17:19:17:49 | github.event.issue.title | Potential expression injection in $@, which may be controlled by an external user. | .github/workflows/comment_issue.yml:17:19:17:49 | github.event.issue.title | github.event.issue.title |
-| .github/workflows/comment_issue.yml:24:31:24:62 | github.event.comment.body | .github/workflows/comment_issue.yml:24:31:24:62 | github.event.comment.body | .github/workflows/comment_issue.yml:24:31:24:62 | github.event.comment.body | Potential expression injection in $@, which may be controlled by an external user. | .github/workflows/comment_issue.yml:24:31:24:62 | github.event.comment.body | github.event.comment.body |
-| .github/workflows/comment_issue.yml:27:31:27:60 | github.event.issue.body | .github/workflows/comment_issue.yml:27:31:27:60 | github.event.issue.body | .github/workflows/comment_issue.yml:27:31:27:60 | github.event.issue.body | Potential expression injection in $@, which may be controlled by an external user. | .github/workflows/comment_issue.yml:27:31:27:60 | github.event.issue.body | github.event.issue.body |
-| .github/workflows/comment_issue.yml:30:31:30:61 | github.event.issue.title | .github/workflows/comment_issue.yml:30:31:30:61 | github.event.issue.title | .github/workflows/comment_issue.yml:30:31:30:61 | github.event.issue.title | Potential expression injection in $@, which may be controlled by an external user. | .github/workflows/comment_issue.yml:30:31:30:61 | github.event.issue.title | github.event.issue.title |
 | .github/workflows/comment_issue_newline.yml:10:25:10:56 | github.event.comment.body | .github/workflows/comment_issue_newline.yml:10:25:10:56 | github.event.comment.body | .github/workflows/comment_issue_newline.yml:10:25:10:56 | github.event.comment.body | Potential expression injection in $@, which may be controlled by an external user. | .github/workflows/comment_issue_newline.yml:10:25:10:56 | github.event.comment.body | github.event.comment.body |
 | .github/workflows/comment_issue_newline.yml:11:24:11:51 | github.event.issue.body | .github/workflows/comment_issue_newline.yml:11:24:11:51 | github.event.issue.body | .github/workflows/comment_issue_newline.yml:11:24:11:51 | github.event.issue.body | Potential expression injection in $@, which may be controlled by an external user. | .github/workflows/comment_issue_newline.yml:11:24:11:51 | github.event.issue.body | github.event.issue.body |
 | .github/workflows/comment_issue_newline.yml:12:24:12:55 | github.event.comment.body | .github/workflows/comment_issue_newline.yml:12:24:12:55 | github.event.comment.body | .github/workflows/comment_issue_newline.yml:12:24:12:55 | github.event.comment.body | Potential expression injection in $@, which may be controlled by an external user. | .github/workflows/comment_issue_newline.yml:12:24:12:55 | github.event.comment.body | github.event.comment.body |
diff --git a/ql/test/query-tests/Security/CWE-094/ExpressionInjection.expected b/ql/test/query-tests/Security/CWE-094/ExpressionInjection.expected
diff --git a/ql/test/query-tests/Security/CWE-918/.github/workflows/test.yml b/ql/test/query-tests/Security/CWE-918/.github/workflows/test.yml
diff --git a/ql/test/query-tests/Security/CWE-918/RequestForgery.expected b/ql/test/query-tests/Security/CWE-918/RequestForgery.expected
diff --git a/ql/test/query-tests/Security/CWE-918/RequestForgery.qlref b/ql/test/query-tests/Security/CWE-918/RequestForgery.qlref

Original file line number	Diff line number	Diff line change
`@@ -86,8 +86,10 @@ predicate externallyDefinedStoreStep(`
`86`	`86`	`)`
`87`	`87`	`}`
`88`	`88`
`89`		`-predicate externallyDefinedSink(DataFlow::ExprNode sink, string kind) {`
	`89`	`+predicate externallyDefinedSink(DataFlow::Node sink, string kind) {`
`90`	`90`	`exists(Uses uses, string action, string version, string input \|`
	`91`	`+ sinkModel(action, version, input, kind) and`
	`92`	`+ uses.getCallee() = action.toLowerCase() and`
`91`	`93`	`(`
`92`	`94`	`if input.trim().matches("env.%")`
`93`	`95`	`then sink.asExpr() = uses.getInScopeEnvVarExpr(input.trim().replaceAll("env.", ""))`
`@@ -96,8 +98,6 @@ predicate externallyDefinedSink(DataFlow::ExprNode sink, string kind) {`
`96`	`98`	`then sink.asExpr() = uses.getArgumentExpr(input.trim().replaceAll("input.", ""))`
`97`	`99`	`else none()`
`98`	`100`	`) and`
`99`		`- sinkModel(action, version, input, kind) and`
`100`		`- uses.getCallee() = action.toLowerCase() and`
`101`	`101`	`(`
`102`	`102`	`if version.trim() = "*"`
`103`	`103`	`then uses.getVersion() = any(string v)`