New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
JS: Add more sources, more unit tests, fixes to the GitHub Actions injection query #12748
base: main
Are you sure you want to change the base?
Conversation
|
QHelp previews: javascript/ql/src/Security/CWE-094/ExpressionInjection.qhelpExpression injection in ActionsUsing user-controlled input in GitHub Actions may lead to code injection in contexts like run: or script:. Code injection in GitHub Actions may allow an attacker to exfiltrate any secrets used in the workflow and the temporary GitHub repository authorization token. The token might have write access to the repository, allowing an attacker to use the token to make changes to the repository. RecommendationThe best practice to avoid code injection vulnerabilities in GitHub workflows is to set the untrusted input value of the expression to an intermediate environment variable and then use the environment variable using the native syntax of the shell/script interpreter (i.e. NOT the ${{ env.VAR }}). It is also recommended to limit the permissions of any tokens used by a workflow such as the GITHUB_TOKEN. ExampleThe following example lets a user inject an arbitrary shell command: on: issue_comment
jobs:
echo-body:
runs-on: ubuntu-latest
steps:
- run: |
echo '${{ github.event.comment.body }}'The following example uses an environment variable, but still allows the injection because of the use of expression syntax: on: issue_comment
jobs:
echo-body:
runs-on: ubuntu-latest
steps:
- env:
BODY: ${{ github.event.issue.body }}
run: |
echo '${{ env.BODY }}'The following example uses shell syntax to read the environment variable and will prevent the attack: on: issue_comment
jobs:
echo-body:
runs-on: ubuntu-latest
steps:
- env:
BODY: ${{ github.event.issue.body }}
run: |
echo '$BODY'References
|
|
Sorry for keep pushing new changes. I think I'm done now. Ready for review. |
|
I made the last commit separate on purpose. Maybe it needs to reverted. I wanted to make the message more clear: instead of injection from |
|
Since nobody is reviewing :) I have added support for composite actions. |
| or | ||
| (exists(on.getNode("discussion")) or exists(on.getNode("discussion_comment"))) and | ||
| isExternalUserControlledDiscussion(context) | ||
| exists(Actions::Env env | isEnvTainted(env, injection, context)) |
Check warning
Code scanning / CodeQL
Omittable 'exists' variable Warning
in this argument
| ( | ||
| injection = context | ||
| or | ||
| exists(Actions::Env env | isEnvTainted(env, injection, context)) |
Check warning
Code scanning / CodeQL
Omittable 'exists' variable Warning
in this argument
| Job getJob() { result = job } | ||
| Job getJob() { result = parent.(Job) } | ||
|
|
||
| Runs getRuns() { result = parent.(Runs) } |
Check warning
Code scanning / CodeQL
Redundant cast Warning
Runs
| /** | ||
| * The env variable name in `${{ env.name }}` | ||
| * is where the external user controlled value was assigned to. | ||
| */ |
Check warning
Code scanning / CodeQL
Predicate QLDoc style. Warning
No description provided.