feat(bash): Improve bash command parsing

Alvaro Muñoz · Alvaro Muñoz · commit 41fade5feb30 · 2024-08-02T12:44:43.000+02:00
diff --git a/ql/lib/codeql/actions/ast/internal/Ast.qll b/ql/lib/codeql/actions/ast/internal/Ast.qll
@@ -1243,7 +1243,7 @@ class RunImpl extends StepImpl {
 
   RunImpl() { this.getNode().lookup("run") = script }
 
-  string getScript() { result = script.getValue() }
+  string getScript() { result = script.getValue().regexpReplaceAll("\\\\\\s*\n", "") }
 
   ScalarValueImpl getScriptScalar() { result = TScalarValueNode(script) }
 
diff --git a/ql/lib/codeql/actions/config/Config.qll b/ql/lib/codeql/actions/config/Config.qll
@@ -92,7 +92,7 @@ predicate argumentInjectionSinksDataModel(string regexp, int command_group, int
   exists(string sub_regexp |
     Extensions::argumentInjectionSinksDataModel(sub_regexp, command_group, argument_group) and
     // capture regexp
-    regexp = ".*" + commandPrefixDelimiter() + sub_regexp + commandSuffixDelimiter() + ".*"
+    regexp = ".*" + commandPrefixDelimiter() + sub_regexp // + commandSuffixDelimiter() +  ".*"
   )
 }
 
diff --git a/ql/test/library-tests/.github/workflows/poisonable_steps.yml b/ql/test/library-tests/.github/workflows/poisonable_steps.yml
@@ -39,3 +39,8 @@ jobs:
       - run: echo "foo" | awk -f ./config.awk > foo.txt
       - run: gcloud builds submit --quiet --substitutions="COMMIT_SHA=foo
       - run: ./foo/cmd
+      - run: |
+          sed -e 's#<branch_to_sync>#TITLE#' \
+              -e 's#<sot_repo>#${{ env.sot_repo }}#' \
+              -e 's#<destination_repo>#${TITLE}#' \
+              .github/workflows/common-copybara.bara.sky.template > .github/workflows/common-copybara.bara.sky
diff --git a/ql/test/library-tests/poisonable_steps.expected b/ql/test/library-tests/poisonable_steps.expected
@@ -30,4 +30,4 @@
 | .github/workflows/poisonable_steps.yml:38:9:39:6 | Run Step |
 | .github/workflows/poisonable_steps.yml:39:9:40:6 | Run Step |
 | .github/workflows/poisonable_steps.yml:40:9:41:6 | Run Step |
-| .github/workflows/poisonable_steps.yml:41:9:41:23 | Run Step |
+| .github/workflows/poisonable_steps.yml:41:9:42:6 | Run Step |
diff --git a/ql/test/library-tests/test.expected b/ql/test/library-tests/test.expected
diff --git a/ql/test/query-tests/Security/CWE-094/.github/workflows/arg_injection.yml b/ql/test/query-tests/Security/CWE-094/.github/workflows/arg_injection.yml
@@ -17,6 +17,14 @@ jobs:
       - run: awk "BEGIN {$TITLE}"
       - run: sed -i "s/git_branch = .*/git_branch = \"$GITHUB_HEAD_REF\"/" config.json
       - run: |
-          # We consider | as a shell pipe so this one is not reported yet until
-          # we can better identify all the commands in a shell script
           sed -i "s|git_branch = .*|git_branch = \"$GITHUB_HEAD_REF\"|" config.json
+      - run: |
+          sed -e 's#<branch_to_sync>#${TITLE}#' \
+              -e 's#<sot_repo>#${{ env.sot_repo }}#' \
+              -e 's#<destination_repo>#TITLE#' \
+              .github/workflows/common-copybara.bara.sky.template > .github/workflows/common-copybara.bara.sky
+      - run: |
+          sed -e 's#<branch_to_sync>#TITLE#' \
+              -e 's#<sot_repo>#${{ env.sot_repo }}#' \
+              -e 's#<destination_repo>#${TITLE}#' \
+              .github/workflows/common-copybara.bara.sky.template > .github/workflows/common-copybara.bara.sky
diff --git a/ql/test/query-tests/Security/CWE-094/ArgumentInjectionCritical.expected b/ql/test/query-tests/Security/CWE-094/ArgumentInjectionCritical.expected
@@ -3,17 +3,25 @@ edges
 | .github/workflows/arg_injection.yml:11:15:11:43 | github.event.issue.title | .github/workflows/arg_injection.yml:15:14:15:52 | echo "foo" \| sed "s/FOO/$TITLE/g" > bar | provenance |  |
 | .github/workflows/arg_injection.yml:11:15:11:43 | github.event.issue.title | .github/workflows/arg_injection.yml:16:14:16:60 | echo $(echo "foo" \| sed "s/FOO/$TITLE/g" > bar) | provenance |  |
 | .github/workflows/arg_injection.yml:11:15:11:43 | github.event.issue.title | .github/workflows/arg_injection.yml:17:14:17:33 | awk "BEGIN {$TITLE}" | provenance |  |
+| .github/workflows/arg_injection.yml:11:15:11:43 | github.event.issue.title | .github/workflows/arg_injection.yml:21:14:25:111 | sed -e 's#<branch_to_sync>#${TITLE}#' \\\n    -e 's#<sot_repo>#${{ env.sot_repo }}#' \\\n    -e 's#<destination_repo>#TITLE#' \\\n    .github/workflows/common-copybara.bara.sky.template > .github/workflows/common-copybara.bara.sky\n | provenance |  |
+| .github/workflows/arg_injection.yml:11:15:11:43 | github.event.issue.title | .github/workflows/arg_injection.yml:26:14:30:111 | sed -e 's#<branch_to_sync>#TITLE#' \\\n    -e 's#<sot_repo>#${{ env.sot_repo }}#' \\\n    -e 's#<destination_repo>#${TITLE}#' \\\n    .github/workflows/common-copybara.bara.sky.template > .github/workflows/common-copybara.bara.sky\n | provenance |  |
 nodes
 | .github/workflows/arg_injection.yml:11:15:11:43 | github.event.issue.title | semmle.label | github.event.issue.title |
 | .github/workflows/arg_injection.yml:14:14:14:33 | sed "s/FOO/$TITLE/g" | semmle.label | sed "s/FOO/$TITLE/g" |
 | .github/workflows/arg_injection.yml:15:14:15:52 | echo "foo" \| sed "s/FOO/$TITLE/g" > bar | semmle.label | echo "foo" \| sed "s/FOO/$TITLE/g" > bar |
 | .github/workflows/arg_injection.yml:16:14:16:60 | echo $(echo "foo" \| sed "s/FOO/$TITLE/g" > bar) | semmle.label | echo $(echo "foo" \| sed "s/FOO/$TITLE/g" > bar) |
 | .github/workflows/arg_injection.yml:17:14:17:33 | awk "BEGIN {$TITLE}" | semmle.label | awk "BEGIN {$TITLE}" |
 | .github/workflows/arg_injection.yml:18:14:18:86 | sed -i "s/git_branch = .*/git_branch = \\"$GITHUB_HEAD_REF\\"/" config.json | semmle.label | sed -i "s/git_branch = .*/git_branch = \\"$GITHUB_HEAD_REF\\"/" config.json |
+| .github/workflows/arg_injection.yml:19:14:20:84 | sed -i "s\|git_branch = .*\|git_branch = \\"$GITHUB_HEAD_REF\\"\|" config.json\n | semmle.label | sed -i "s\|git_branch = .*\|git_branch = \\"$GITHUB_HEAD_REF\\"\|" config.json\n |
+| .github/workflows/arg_injection.yml:21:14:25:111 | sed -e 's#<branch_to_sync>#${TITLE}#' \\\n    -e 's#<sot_repo>#${{ env.sot_repo }}#' \\\n    -e 's#<destination_repo>#TITLE#' \\\n    .github/workflows/common-copybara.bara.sky.template > .github/workflows/common-copybara.bara.sky\n | semmle.label | sed -e 's#<branch_to_sync>#${TITLE}#' \\\n    -e 's#<sot_repo>#${{ env.sot_repo }}#' \\\n    -e 's#<destination_repo>#TITLE#' \\\n    .github/workflows/common-copybara.bara.sky.template > .github/workflows/common-copybara.bara.sky\n |
+| .github/workflows/arg_injection.yml:26:14:30:111 | sed -e 's#<branch_to_sync>#TITLE#' \\\n    -e 's#<sot_repo>#${{ env.sot_repo }}#' \\\n    -e 's#<destination_repo>#${TITLE}#' \\\n    .github/workflows/common-copybara.bara.sky.template > .github/workflows/common-copybara.bara.sky\n | semmle.label | sed -e 's#<branch_to_sync>#TITLE#' \\\n    -e 's#<sot_repo>#${{ env.sot_repo }}#' \\\n    -e 's#<destination_repo>#${TITLE}#' \\\n    .github/workflows/common-copybara.bara.sky.template > .github/workflows/common-copybara.bara.sky\n |
 subpaths
 #select
 | .github/workflows/arg_injection.yml:14:14:14:33 | sed "s/FOO/$TITLE/g" | .github/workflows/arg_injection.yml:11:15:11:43 | github.event.issue.title | .github/workflows/arg_injection.yml:14:14:14:33 | sed "s/FOO/$TITLE/g" | Potential argument injection in $@ command, which may be controlled by an external user. | .github/workflows/arg_injection.yml:14:14:14:33 | sed "s/FOO/$TITLE/g" | sed |
 | .github/workflows/arg_injection.yml:15:14:15:52 | echo "foo" \| sed "s/FOO/$TITLE/g" > bar | .github/workflows/arg_injection.yml:11:15:11:43 | github.event.issue.title | .github/workflows/arg_injection.yml:15:14:15:52 | echo "foo" \| sed "s/FOO/$TITLE/g" > bar | Potential argument injection in $@ command, which may be controlled by an external user. | .github/workflows/arg_injection.yml:15:14:15:52 | echo "foo" \| sed "s/FOO/$TITLE/g" > bar | sed |
 | .github/workflows/arg_injection.yml:16:14:16:60 | echo $(echo "foo" \| sed "s/FOO/$TITLE/g" > bar) | .github/workflows/arg_injection.yml:11:15:11:43 | github.event.issue.title | .github/workflows/arg_injection.yml:16:14:16:60 | echo $(echo "foo" \| sed "s/FOO/$TITLE/g" > bar) | Potential argument injection in $@ command, which may be controlled by an external user. | .github/workflows/arg_injection.yml:16:14:16:60 | echo $(echo "foo" \| sed "s/FOO/$TITLE/g" > bar) | sed |
 | .github/workflows/arg_injection.yml:17:14:17:33 | awk "BEGIN {$TITLE}" | .github/workflows/arg_injection.yml:11:15:11:43 | github.event.issue.title | .github/workflows/arg_injection.yml:17:14:17:33 | awk "BEGIN {$TITLE}" | Potential argument injection in $@ command, which may be controlled by an external user. | .github/workflows/arg_injection.yml:17:14:17:33 | awk "BEGIN {$TITLE}" | awk |
 | .github/workflows/arg_injection.yml:18:14:18:86 | sed -i "s/git_branch = .*/git_branch = \\"$GITHUB_HEAD_REF\\"/" config.json | .github/workflows/arg_injection.yml:18:14:18:86 | sed -i "s/git_branch = .*/git_branch = \\"$GITHUB_HEAD_REF\\"/" config.json | .github/workflows/arg_injection.yml:18:14:18:86 | sed -i "s/git_branch = .*/git_branch = \\"$GITHUB_HEAD_REF\\"/" config.json | Potential argument injection in $@ command, which may be controlled by an external user. | .github/workflows/arg_injection.yml:18:14:18:86 | sed -i "s/git_branch = .*/git_branch = \\"$GITHUB_HEAD_REF\\"/" config.json | sed |
+| .github/workflows/arg_injection.yml:19:14:20:84 | sed -i "s\|git_branch = .*\|git_branch = \\"$GITHUB_HEAD_REF\\"\|" config.json\n | .github/workflows/arg_injection.yml:19:14:20:84 | sed -i "s\|git_branch = .*\|git_branch = \\"$GITHUB_HEAD_REF\\"\|" config.json\n | .github/workflows/arg_injection.yml:19:14:20:84 | sed -i "s\|git_branch = .*\|git_branch = \\"$GITHUB_HEAD_REF\\"\|" config.json\n | Potential argument injection in $@ command, which may be controlled by an external user. | .github/workflows/arg_injection.yml:19:14:20:84 | sed -i "s\|git_branch = .*\|git_branch = \\"$GITHUB_HEAD_REF\\"\|" config.json\n | sed |
+| .github/workflows/arg_injection.yml:21:14:25:111 | sed -e 's#<branch_to_sync>#${TITLE}#' \\\n    -e 's#<sot_repo>#${{ env.sot_repo }}#' \\\n    -e 's#<destination_repo>#TITLE#' \\\n    .github/workflows/common-copybara.bara.sky.template > .github/workflows/common-copybara.bara.sky\n | .github/workflows/arg_injection.yml:11:15:11:43 | github.event.issue.title | .github/workflows/arg_injection.yml:21:14:25:111 | sed -e 's#<branch_to_sync>#${TITLE}#' \\\n    -e 's#<sot_repo>#${{ env.sot_repo }}#' \\\n    -e 's#<destination_repo>#TITLE#' \\\n    .github/workflows/common-copybara.bara.sky.template > .github/workflows/common-copybara.bara.sky\n | Potential argument injection in $@ command, which may be controlled by an external user. | .github/workflows/arg_injection.yml:21:14:25:111 | sed -e 's#<branch_to_sync>#${TITLE}#' \\\n    -e 's#<sot_repo>#${{ env.sot_repo }}#' \\\n    -e 's#<destination_repo>#TITLE#' \\\n    .github/workflows/common-copybara.bara.sky.template > .github/workflows/common-copybara.bara.sky\n | sed |
+| .github/workflows/arg_injection.yml:26:14:30:111 | sed -e 's#<branch_to_sync>#TITLE#' \\\n    -e 's#<sot_repo>#${{ env.sot_repo }}#' \\\n    -e 's#<destination_repo>#${TITLE}#' \\\n    .github/workflows/common-copybara.bara.sky.template > .github/workflows/common-copybara.bara.sky\n | .github/workflows/arg_injection.yml:11:15:11:43 | github.event.issue.title | .github/workflows/arg_injection.yml:26:14:30:111 | sed -e 's#<branch_to_sync>#TITLE#' \\\n    -e 's#<sot_repo>#${{ env.sot_repo }}#' \\\n    -e 's#<destination_repo>#${TITLE}#' \\\n    .github/workflows/common-copybara.bara.sky.template > .github/workflows/common-copybara.bara.sky\n | Potential argument injection in $@ command, which may be controlled by an external user. | .github/workflows/arg_injection.yml:26:14:30:111 | sed -e 's#<branch_to_sync>#TITLE#' \\\n    -e 's#<sot_repo>#${{ env.sot_repo }}#' \\\n    -e 's#<destination_repo>#${TITLE}#' \\\n    .github/workflows/common-copybara.bara.sky.template > .github/workflows/common-copybara.bara.sky\n | sed |
diff --git a/ql/test/query-tests/Security/CWE-094/ArgumentInjectionMedium.expected b/ql/test/query-tests/Security/CWE-094/ArgumentInjectionMedium.expected
@@ -3,12 +3,17 @@ edges
 | .github/workflows/arg_injection.yml:11:15:11:43 | github.event.issue.title | .github/workflows/arg_injection.yml:15:14:15:52 | echo "foo" \| sed "s/FOO/$TITLE/g" > bar | provenance |  |
 | .github/workflows/arg_injection.yml:11:15:11:43 | github.event.issue.title | .github/workflows/arg_injection.yml:16:14:16:60 | echo $(echo "foo" \| sed "s/FOO/$TITLE/g" > bar) | provenance |  |
 | .github/workflows/arg_injection.yml:11:15:11:43 | github.event.issue.title | .github/workflows/arg_injection.yml:17:14:17:33 | awk "BEGIN {$TITLE}" | provenance |  |
+| .github/workflows/arg_injection.yml:11:15:11:43 | github.event.issue.title | .github/workflows/arg_injection.yml:21:14:25:111 | sed -e 's#<branch_to_sync>#${TITLE}#' \\\n    -e 's#<sot_repo>#${{ env.sot_repo }}#' \\\n    -e 's#<destination_repo>#TITLE#' \\\n    .github/workflows/common-copybara.bara.sky.template > .github/workflows/common-copybara.bara.sky\n | provenance |  |
+| .github/workflows/arg_injection.yml:11:15:11:43 | github.event.issue.title | .github/workflows/arg_injection.yml:26:14:30:111 | sed -e 's#<branch_to_sync>#TITLE#' \\\n    -e 's#<sot_repo>#${{ env.sot_repo }}#' \\\n    -e 's#<destination_repo>#${TITLE}#' \\\n    .github/workflows/common-copybara.bara.sky.template > .github/workflows/common-copybara.bara.sky\n | provenance |  |
 nodes
 | .github/workflows/arg_injection.yml:11:15:11:43 | github.event.issue.title | semmle.label | github.event.issue.title |
 | .github/workflows/arg_injection.yml:14:14:14:33 | sed "s/FOO/$TITLE/g" | semmle.label | sed "s/FOO/$TITLE/g" |
 | .github/workflows/arg_injection.yml:15:14:15:52 | echo "foo" \| sed "s/FOO/$TITLE/g" > bar | semmle.label | echo "foo" \| sed "s/FOO/$TITLE/g" > bar |
 | .github/workflows/arg_injection.yml:16:14:16:60 | echo $(echo "foo" \| sed "s/FOO/$TITLE/g" > bar) | semmle.label | echo $(echo "foo" \| sed "s/FOO/$TITLE/g" > bar) |
 | .github/workflows/arg_injection.yml:17:14:17:33 | awk "BEGIN {$TITLE}" | semmle.label | awk "BEGIN {$TITLE}" |
 | .github/workflows/arg_injection.yml:18:14:18:86 | sed -i "s/git_branch = .*/git_branch = \\"$GITHUB_HEAD_REF\\"/" config.json | semmle.label | sed -i "s/git_branch = .*/git_branch = \\"$GITHUB_HEAD_REF\\"/" config.json |
+| .github/workflows/arg_injection.yml:19:14:20:84 | sed -i "s\|git_branch = .*\|git_branch = \\"$GITHUB_HEAD_REF\\"\|" config.json\n | semmle.label | sed -i "s\|git_branch = .*\|git_branch = \\"$GITHUB_HEAD_REF\\"\|" config.json\n |
+| .github/workflows/arg_injection.yml:21:14:25:111 | sed -e 's#<branch_to_sync>#${TITLE}#' \\\n    -e 's#<sot_repo>#${{ env.sot_repo }}#' \\\n    -e 's#<destination_repo>#TITLE#' \\\n    .github/workflows/common-copybara.bara.sky.template > .github/workflows/common-copybara.bara.sky\n | semmle.label | sed -e 's#<branch_to_sync>#${TITLE}#' \\\n    -e 's#<sot_repo>#${{ env.sot_repo }}#' \\\n    -e 's#<destination_repo>#TITLE#' \\\n    .github/workflows/common-copybara.bara.sky.template > .github/workflows/common-copybara.bara.sky\n |
+| .github/workflows/arg_injection.yml:26:14:30:111 | sed -e 's#<branch_to_sync>#TITLE#' \\\n    -e 's#<sot_repo>#${{ env.sot_repo }}#' \\\n    -e 's#<destination_repo>#${TITLE}#' \\\n    .github/workflows/common-copybara.bara.sky.template > .github/workflows/common-copybara.bara.sky\n | semmle.label | sed -e 's#<branch_to_sync>#TITLE#' \\\n    -e 's#<sot_repo>#${{ env.sot_repo }}#' \\\n    -e 's#<destination_repo>#${TITLE}#' \\\n    .github/workflows/common-copybara.bara.sky.template > .github/workflows/common-copybara.bara.sky\n |
 subpaths
 #select

Original file line number	Diff line number	Diff line change
`@@ -92,7 +92,7 @@ predicate argumentInjectionSinksDataModel(string regexp, int command_group, int`
`92`	`92`	`exists(string sub_regexp \|`
`93`	`93`	`Extensions::argumentInjectionSinksDataModel(sub_regexp, command_group, argument_group) and`
`94`	`94`	`// capture regexp`
`95`		`- regexp = "." + commandPrefixDelimiter() + sub_regexp + commandSuffixDelimiter() + "."`
	`95`	`+ regexp = "." + commandPrefixDelimiter() + sub_regexp // + commandSuffixDelimiter() + "."`
`96`	`96`	`)`
`97`	`97`	`}`
`98`	`98`