Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,12 @@ ql/actions/ql/src/experimental/Security/CWE-078/CommandInjectionCritical.ql
ql/actions/ql/src/experimental/Security/CWE-078/CommandInjectionMedium.ql
ql/actions/ql/src/experimental/Security/CWE-088/ArgumentInjectionCritical.ql
ql/actions/ql/src/experimental/Security/CWE-088/ArgumentInjectionMedium.ql
ql/actions/ql/src/experimental/Security/CWE-183/UnsoundContains.ql
ql/actions/ql/src/experimental/Security/CWE-200/SecretExfiltration.ql
ql/actions/ql/src/experimental/Security/CWE-200/SecretsInherit.ql
ql/actions/ql/src/experimental/Security/CWE-284/CodeExecutionOnSelfHostedRunner.ql
ql/actions/ql/src/experimental/Security/CWE-290/SpoofableActorCheck.ql
ql/actions/ql/src/experimental/Security/CWE-798/HardcodedContainerCredentials.ql
ql/actions/ql/src/experimental/Security/CWE-829/ArtifactPoisoningPathTraversal.ql
ql/actions/ql/src/experimental/Security/CWE-829/UnversionedImmutableAction.ql
ql/actions/ql/src/experimental/Security/CWE-918/RequestForgery.ql
42 changes: 42 additions & 0 deletions actions/ql/src/experimental/Security/CWE-183/UnsoundContains.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
## Overview

The GitHub Actions `contains()` function behaves differently depending on the type of its first argument. When the first argument is a string, `contains()` performs a substring match rather than an exact membership check. This can be bypassed by an attacker who crafts a value that happens to be a substring of the target string.

For example, the condition `contains('refs/heads/main refs/heads/develop', github.ref)` would also match `github.ref` values like `refs/heads/mai` or `refs/heads/evelop`, because these are substrings of the target string.

## Recommendation

Use `fromJSON()` to pass an array as the first argument to `contains()`, which performs an exact array membership check:

```yaml
if: contains(fromJSON('["refs/heads/main", "refs/heads/develop"]'), github.ref)
```

Alternatively, use explicit equality checks combined with logical OR:

```yaml
if: github.ref == 'refs/heads/main' || github.ref == 'refs/heads/develop'
```

## Example

### Incorrect Usage

```yaml
steps:
- run: terraform apply
if: contains('refs/heads/main refs/heads/develop', github.ref)
```

### Correct Usage

```yaml
steps:
- run: terraform apply
if: contains(fromJSON('["refs/heads/main", "refs/heads/develop"]'), github.ref)
```

## References

- GitHub Docs: [contains() function](https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/evaluate-expressions-in-workflows-and-actions#contains).
- Zizmor: [unsound-contains](https://docs.zizmor.sh/audits/#unsound-contains).
41 changes: 41 additions & 0 deletions actions/ql/src/experimental/Security/CWE-183/UnsoundContains.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/**
* @name Unsound use of contains() for membership check
* @description Using `contains()` with a string literal as the first argument performs a substring
* match instead of an array membership check, which can be bypassed.
* @kind problem
* @precision high
* @security-severity 5.0
* @problem.severity warning
* @id actions/unsound-contains
* @tags actions
* security
* experimental
* external/cwe/cwe-183
*/

import actions

/**
* Holds if `expr` is an expression within an `if:` condition that uses `contains()`
* with a string literal as the first argument, performing a substring match
* instead of an array membership check.
*/
predicate isUnsoundContains(Expression expr) {
exists(If ifNode |
ifNode.getConditionExpr() = expr and
// Match contains() with a string-literal first argument
// This catches: contains('refs/heads/main refs/heads/develop', github.ref)
// But NOT: contains(fromJSON('["refs/heads/main"]'), github.ref)
// And NOT: contains(github.event.issue.labels.*.name, 'bug')
(
expr.getExpression().regexpMatch("(?i)contains\\s*\\(\\s*'[^']*'\\s*,.*")
or
expr.getExpression().regexpMatch("(?i)contains\\s*\\(\\s*\"[^\"]*\"\\s*,.*")
)
)
}

from Expression expr
where isUnsoundContains(expr)
select expr,
"The `contains()` call performs a substring match which can be bypassed. Use `fromJSON()` to create an array for a proper membership check."
33 changes: 33 additions & 0 deletions actions/ql/src/experimental/Security/CWE-200/SecretsInherit.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
## Overview

When calling a reusable workflow with `secrets: inherit`, every secret the calling workflow can access (organization, repository, and environment secrets) is forwarded to the callee. This is convenient but broader than most callees require. If the reusable workflow has a vulnerability — for example, a template-injection flaw — the blast radius includes every inherited secret rather than just the ones it actually uses.

## Recommendation

As a defense-in-depth measure, prefer passing only the secrets the reusable workflow needs via an explicit `secrets:` block.

## Example

### Incorrect Usage

```yaml
jobs:
call-workflow:
uses: org/repo/.github/workflows/reusable.yml@main
secrets: inherit
```
### Correct Usage
```yaml
jobs:
call-workflow:
uses: org/repo/.github/workflows/reusable.yml@main
secrets:
API_KEY: ${{ secrets.API_KEY }}
```
## References
- GitHub Docs: [Passing secrets to reusable workflows](https://docs.github.com/en/actions/sharing-automations/reusing-workflows#passing-inputs-and-secrets-to-a-reusable-workflow).
- Zizmor: [secrets-inherit](https://docs.zizmor.sh/audits/#secrets-inherit).
26 changes: 26 additions & 0 deletions actions/ql/src/experimental/Security/CWE-200/SecretsInherit.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/**
* @name Secrets inherited by reusable workflow
* @description Using `secrets: inherit` passes every secret the calling workflow can access
* to a reusable workflow, which is more than most callees need.
* @kind problem
* @precision medium
* @security-severity 3.0
* @problem.severity recommendation
* @id actions/secrets-inherit
* @tags actions
* security
* experimental
* external/cwe/cwe-200
*/

import actions
private import codeql.actions.ast.internal.Yaml
private import codeql.actions.ast.internal.Ast

from ExternalJob job, YamlScalar secretsNode
where
secretsNode = job.(ExternalJobImpl).getNode().lookup("secrets") and
secretsNode.getValue() = "inherit"
select secretsNode,
"Every secret accessible to the calling workflow is forwarded to $@. Consider passing only the secrets it actually needs.",
job.(Uses).getCalleeNode(), job.(Uses).getCallee()
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
## Overview

Many workflows use `github.actor` or `github.triggering_actor` to check if a specific bot (such as Dependabot or Renovate) triggered the workflow, and then bypass security checks or perform privileged actions. However, `github.actor` refers to the last actor to perform an "action" on the triggering context, not necessarily the actor that actually caused the trigger.

An attacker can exploit this by creating a pull request where the workflow run's `github.actor` is `'dependabot[bot]'` (for example, because Dependabot was the latest actor on the PR), but the branch contains attacker-controlled code, bypassing the actor check.

## Recommendation

Instead of checking `github.actor`, use a context that refers to the actor who created the triggering event. For `pull_request_target` workflows, use `github.event.pull_request.user.login`. For `issue_comment` workflows, use `github.event.comment.user.login`.

More generally, consider whether a bot-bypass check is the right approach. GitHub's documentation recommends not using `pull_request_target` for auto-merge workflows.

## Example

### Incorrect Usage

```yaml
on: pull_request_target

jobs:
automerge:
runs-on: ubuntu-latest
if: github.actor == 'dependabot[bot]'
steps:
- run: gh pr merge --auto --merge "$PR_URL"
env:
PR_URL: ${{ github.event.pull_request.html_url }}
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
```

### Correct Usage

```yaml
on: pull_request_target

jobs:
automerge:
runs-on: ubuntu-latest
if: github.event.pull_request.user.login == 'dependabot[bot]'
steps:
- run: gh pr merge --auto --merge "$PR_URL"
env:
PR_URL: ${{ github.event.pull_request.html_url }}
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
```

## References

- Synacktiv: [GitHub Actions exploitations: Dependabot](https://www.synacktiv.com/publications/github-actions-exploitation-dependabot).
- GitHub Docs: [Automating Dependabot with GitHub Actions](https://docs.github.com/en/code-security/dependabot/working-with-dependabot/automating-dependabot-with-github-actions).
- Zizmor: [bot-conditions](https://docs.zizmor.sh/audits/#bot-conditions).
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/**
* @name Spoofable actor check used as security control
* @description Checking `github.actor` or `github.triggering_actor` against a bot name
* is spoofable and should not be used as a security control.
* @kind problem
* @precision high
* @security-severity 7.0
* @problem.severity warning
* @id actions/spoofable-actor-check
* @tags actions
* security
* experimental
* external/cwe/cwe-290
*/

import actions

/**
* Holds if `ifNode` contains a spoofable bot actor check.
*
* Matches conditions like:
* `github.actor == 'dependabot[bot]'`
* `github.triggering_actor == 'renovate[bot]'`
* `'dependabot[bot]' == github.actor`
*
* These are spoofable because `github.actor` refers to the last actor
* to act on the triggering context, not necessarily the actor that
* caused the trigger.
*/
predicate isSpoofableBotCheck(If ifNode) {
exists(string cond |
cond = normalizeExpr(ifNode.getCondition()) and
(
// github.actor == 'something[bot]' or github.triggering_actor == 'something[bot]'
cond.regexpMatch("(?s).*\\bgithub\\.(actor|triggering_actor)\\s*==\\s*'[^']*\\[bot\\][^']*'.*")
or
// reversed: 'something[bot]' == github.actor
cond.regexpMatch("(?s).*'[^']*\\[bot\\][^']*'\\s*==\\s*github\\.(actor|triggering_actor)\\b.*")
)
)
}

from If ifNode, Event event
where
isSpoofableBotCheck(ifNode) and
event = ifNode.getATriggerEvent() and
event.isExternallyTriggerable()
select ifNode,
"This condition checks `github.actor` against a bot name, which is spoofable on $@ triggers. Use `github.event.pull_request.user.login` or similar non-spoofable context instead.",
event, event.getName()
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
## Overview

Hardcoding credentials (passwords) in GitHub Actions workflow `container` or `services` configurations embeds secrets directly in the repository source code. Anyone with read access to the repository can see these credentials.

## Recommendation

Use [encrypted secrets](https://docs.github.com/en/actions/security-for-github-actions/security-guides/using-secrets-in-github-actions) instead of hardcoded credentials.

## Example

### Incorrect Usage

```yaml
jobs:
test:
runs-on: ubuntu-latest
container:
image: registry.example.com/app
credentials:
username: user
password: hackme
steps:
- run: echo 'hello'
```

### Correct Usage

```yaml
jobs:
test:
runs-on: ubuntu-latest
container:
image: registry.example.com/app
credentials:
username: ${{ secrets.REGISTRY_USERNAME }}
password: ${{ secrets.REGISTRY_PASSWORD }}
steps:
- run: echo 'hello'
```

## References

- GitHub Docs: [Using encrypted secrets in a workflow](https://docs.github.com/en/actions/security-for-github-actions/security-guides/using-secrets-in-github-actions).
- Zizmor: [hardcoded-container-credentials](https://docs.zizmor.sh/audits/#hardcoded-container-credentials).
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/**
* @name Hardcoded credentials in container configuration
* @description Hardcoding credentials in workflow container or service configurations
* exposes secrets in the repository source code.
* @kind problem
* @precision high
* @security-severity 9.0
* @problem.severity error
* @id actions/hardcoded-container-credentials
* @tags actions
* security
* experimental
* external/cwe/cwe-798
*/

import actions
private import codeql.actions.ast.internal.Yaml
private import codeql.actions.ast.internal.Ast

/**
* Gets a `credentials.password` scalar node from a container or service mapping within a job.
*/
YamlScalar getAHardcodedPassword(LocalJobImpl job, string context) {
exists(YamlMapping creds |
// Job-level container credentials
creds = job.getNode().lookup("container").(YamlMapping).lookup("credentials") and
context = "container"
or
// Service-level container credentials
exists(YamlMapping service |
service = job.getNode().lookup("services").(YamlMapping).lookup(_) and
creds = service.lookup("credentials") and
context = "service"
)
|
result = creds.lookup("password") and
// Not a ${{ }} expression reference (e.g. ${{ secrets.PASSWORD }})
not result.getValue().regexpMatch("\\$\\{\\{.*\\}\\}")
)
}

from LocalJob job, YamlScalar password, string context
where password = getAHardcodedPassword(job, context)
select password,
"Hardcoded password in " + context + " credentials for job $@. Use an encrypted secret instead.",
job, job.(Job).getId()
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
name: unsound-contains-test
on: push

jobs:
deploy:
runs-on: ubuntu-latest
steps:
# Positive: contains() with space-separated string (substring match)
- run: terraform apply
if: contains('refs/heads/main refs/heads/develop', github.ref)

# Positive: contains() with double-quoted string
- run: terraform apply
if: contains("refs/heads/main", github.ref)

# Negative: contains() with fromJSON (proper array membership)
- run: terraform apply
if: contains(fromJSON('["refs/heads/main", "refs/heads/develop"]'), github.ref)

# Negative: contains() with context as first arg (array membership on labels)
- run: echo "has bug label"
if: contains(github.event.issue.labels.*.name, 'bug')

# Negative: no contains() at all
- run: terraform apply
if: github.ref == 'refs/heads/main'
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
| .github/workflows/unsound-contains.yml:10:15:10:72 | contains('refs/heads/main refs/heads/develop', github.ref) | The `contains()` call performs a substring match which can be bypassed. Use `fromJSON()` to create an array for a proper membership check. |
| .github/workflows/unsound-contains.yml:14:15:14:53 | contains("refs/heads/main", github.ref) | The `contains()` call performs a substring match which can be bypassed. Use `fromJSON()` to create an array for a proper membership check. |
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
experimental/Security/CWE-183/UnsoundContains.ql

Check warning

Code scanning / CodeQL

Query test without inline test expectations Warning test

Query test does not use inline test expectations.
Loading
Loading