hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2025-12-28 22:56:32 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #36 from GitHubSecurityLab/fix_source_regexps

Browse Source 
fix(fn): Apply json wrappers to source regexps
This commit is contained in:
Alvaro Muñoz
2024-03-15 14:05:29 +01:00
committed by GitHub
parent 27a9bc8564 d9e589c6e7
commit 0da8f8d299
6 changed files with 118 additions and 50 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
ql/lib/codeql/actions/Ast.qll
View File

@@ -9,6 +9,15 @@ module Utils {
.regexpReplaceAll("\\[\"([a-zA-Z0-9_\\*\\-]+)\"\\]", ".$1")
.regexpReplaceAll("\\s*\\.\\s*", ".")
}
bindingset[regex]
string wrapRegexp(string regex) {
result =
[
"\\b" + regex + "\\b", "fromJSON\\(\\s*" + regex + "\\s*\\)",
"toJSON\\(\\s*" + regex + "\\s*\\)"
]
}
}
class AstNode instanceof AstNodeImpl {

18
ql/lib/codeql/actions/ast/internal/Ast.qll
View File

@@ -813,28 +813,24 @@ abstract class SimpleReferenceExpressionImpl extends ExpressionImpl {
}
private string stepsCtxRegex() {
result = wrapRegexp("steps\\.([A-Za-z0-9_-]+)\\.outputs\\.([A-Za-z0-9_-]+)")
result = Utils::wrapRegexp("steps\\.([A-Za-z0-9_-]+)\\.outputs\\.([A-Za-z0-9_-]+)")
}
private string needsCtxRegex() {
result = wrapRegexp("needs\\.([A-Za-z0-9_-]+)\\.outputs\\.([A-Za-z0-9_-]+)")
result = Utils::wrapRegexp("needs\\.([A-Za-z0-9_-]+)\\.outputs\\.([A-Za-z0-9_-]+)")
}
private string jobsCtxRegex() {
result = wrapRegexp("jobs\\.([A-Za-z0-9_-]+)\\.outputs\\.([A-Za-z0-9_-]+)")
result = Utils::wrapRegexp("jobs\\.([A-Za-z0-9_-]+)\\.outputs\\.([A-Za-z0-9_-]+)")
}
private string envCtxRegex() { result = wrapRegexp("env\\.([A-Za-z0-9_-]+)") }
private string envCtxRegex() { result = Utils::wrapRegexp("env\\.([A-Za-z0-9_-]+)") }
private string matrixCtxRegex() { result = wrapRegexp("matrix\\.([A-Za-z0-9_-]+)") }
private string matrixCtxRegex() { result = Utils::wrapRegexp("matrix\\.([A-Za-z0-9_-]+)") }
private string inputsCtxRegex() {
result = wrapRegexp(["inputs\\.([A-Za-z0-9_-]+)", "github\\.event\\.inputs\\.([A-Za-z0-9_-]+)"])
}
bindingset[regex]
private string wrapRegexp(string regex) {
result = ["\\b" + regex + "\\b", "fromJSON\\(" + regex + "\\)", "toJSON\\(" + regex + "\\)"]
result =
Utils::wrapRegexp(["inputs\\.([A-Za-z0-9_-]+)", "github\\.event\\.inputs\\.([A-Za-z0-9_-]+)"])
}
/**

74
ql/lib/codeql/actions/dataflow/FlowSources.qll
View File

@@ -25,10 +25,8 @@ abstract class RemoteFlowSource extends SourceNode {
bindingset[context]
private predicate isExternalUserControlledIssue(string context) {
exists(string reg |
reg = ["\\bgithub\\.event\\.issue\\.title\\b", "\\bgithub\\.event\\.issue\\.body\\b"]
|
Utils::normalizeExpr(context).regexpMatch(reg)
exists(string reg | reg = ["github\\.event\\.issue\\.title", "github\\.event\\.issue\\.body"] |
Utils::normalizeExpr(context).regexpMatch(Utils::wrapRegexp(reg))
)
}
@@ -37,26 +35,26 @@ private predicate isExternalUserControlledPullRequest(string context) {
exists(string reg |
reg =
[
"\\bgithub\\.event\\.pull_request\\.title\\b", "\\bgithub\\.event\\.pull_request\\.body\\b",
"\\bgithub\\.event\\.pull_request\\.head\\.label\\b",
"\\bgithub\\.event\\.pull_request\\.head\\.repo\\.default_branch\\b",
"\\bgithub\\.event\\.pull_request\\.head\\.repo\\.description\\b",
"\\bgithub\\.event\\.pull_request\\.head\\.repo\\.homepage\\b",
"\\bgithub\\.event\\.pull_request\\.head\\.ref\\b", "\\bgithub\\.head_ref\\b"
"github\\.event\\.pull_request\\.title", "github\\.event\\.pull_request\\.body",
"github\\.event\\.pull_request\\.head\\.label",
"github\\.event\\.pull_request\\.head\\.repo\\.default_branch",
"github\\.event\\.pull_request\\.head\\.repo\\.description",
"github\\.event\\.pull_request\\.head\\.repo\\.homepage",
"github\\.event\\.pull_request\\.head\\.ref", "github\\.head_ref"
]
|
Utils::normalizeExpr(context).regexpMatch(reg)
Utils::normalizeExpr(context).regexpMatch(Utils::wrapRegexp(reg))
)
}
bindingset[context]
private predicate isExternalUserControlledReview(string context) {
Utils::normalizeExpr(context).regexpMatch("\\bgithub\\.event\\.review\\.body\\b")
Utils::normalizeExpr(context).regexpMatch(Utils::wrapRegexp("github\\.event\\.review\\.body"))
}
bindingset[context]
private predicate isExternalUserControlledComment(string context) {
Utils::normalizeExpr(context).regexpMatch("\\bgithub\\.event\\.comment\\.body\\b")
Utils::normalizeExpr(context).regexpMatch(Utils::wrapRegexp("github\\.event\\.comment\\.body"))
}
bindingset[context]
@@ -64,11 +62,11 @@ private predicate isExternalUserControlledGollum(string context) {
exists(string reg |
reg =
[
"\\bgithub\\.event\\.pages\\[[0-9]+\\]\\.page_name\\b",
"\\bgithub\\.event\\.pages\\[[0-9]+\\]\\.title\\b"
"github\\.event\\.pages\\[[0-9]+\\]\\.page_name",
"github\\.event\\.pages\\[[0-9]+\\]\\.title"
]
|
Utils::normalizeExpr(context).regexpMatch(reg)
Utils::normalizeExpr(context).regexpMatch(Utils::wrapRegexp(reg))
)
}
@@ -77,28 +75,27 @@ private predicate isExternalUserControlledCommit(string context) {
exists(string reg |
reg =
[
"\\bgithub\\.event\\.commits\\[[0-9]+\\]\\.message\\b",
"\\bgithub\\.event\\.head_commit\\.message\\b",
"\\bgithub\\.event\\.head_commit\\.author\\.email\\b",
"\\bgithub\\.event\\.head_commit\\.author\\.name\\b",
"\\bgithub\\.event\\.head_commit\\.committer\\.email\\b",
"\\bgithub\\.event\\.head_commit\\.committer\\.name\\b",
"\\bgithub\\.event\\.commits\\[[0-9]+\\]\\.author\\.email\\b",
"\\bgithub\\.event\\.commits\\[[0-9]+\\]\\.author\\.name\\b",
"\\bgithub\\.event\\.commits\\[[0-9]+\\]\\.committer\\.email\\b",
"\\bgithub\\.event\\.commits\\[[0-9]+\\]\\.committer\\.name\\b",
"github\\.event\\.commits\\[[0-9]+\\]\\.message", "github\\.event\\.head_commit\\.message",
"github\\.event\\.head_commit\\.author\\.email",
"github\\.event\\.head_commit\\.author\\.name",
"github\\.event\\.head_commit\\.committer\\.email",
"github\\.event\\.head_commit\\.committer\\.name",
"github\\.event\\.commits\\[[0-9]+\\]\\.author\\.email",
"github\\.event\\.commits\\[[0-9]+\\]\\.author\\.name",
"github\\.event\\.commits\\[[0-9]+\\]\\.committer\\.email",
"github\\.event\\.commits\\[[0-9]+\\]\\.committer\\.name",
]
|
Utils::normalizeExpr(context).regexpMatch(reg)
Utils::normalizeExpr(context).regexpMatch(Utils::wrapRegexp(reg))
)
}
bindingset[context]
private predicate isExternalUserControlledDiscussion(string context) {
exists(string reg |
reg = ["\\bgithub\\.event\\.discussion\\.title\\b", "\\bgithub\\.event\\.discussion\\.body\\b"]
reg = ["github\\.event\\.discussion\\.title", "github\\.event\\.discussion\\.body"]
|
Utils::normalizeExpr(context).regexpMatch(reg)
Utils::normalizeExpr(context).regexpMatch(Utils::wrapRegexp(reg))
)
}
@@ -107,18 +104,17 @@ private predicate isExternalUserControlledWorkflowRun(string context) {
exists(string reg |
reg =
[
"\\bgithub\\.event\\.workflow\\.path\\b",
"\\bgithub\\.event\\.workflow_run\\.head_branch\\b",
"\\bgithub\\.event\\.workflow_run\\.display_title\\b",
"\\bgithub\\.event\\.workflow_run\\.head_repository\\.description\\b",
"\\bgithub\\.event\\.workflow_run\\.head_commit\\.message\\b",
"\\bgithub\\.event\\.workflow_run\\.head_commit\\.author\\.email\\b",
"\\bgithub\\.event\\.workflow_run\\.head_commit\\.author\\.name\\b",
"\\bgithub\\.event\\.workflow_run\\.head_commit\\.committer\\.email\\b",
"\\bgithub\\.event\\.workflow_run\\.head_commit\\.committer\\.name\\b",
"github\\.event\\.workflow\\.path", "github\\.event\\.workflow_run\\.head_branch",
"github\\.event\\.workflow_run\\.display_title",
"github\\.event\\.workflow_run\\.head_repository\\.description",
"github\\.event\\.workflow_run\\.head_commit\\.message",
"github\\.event\\.workflow_run\\.head_commit\\.author\\.email",
"github\\.event\\.workflow_run\\.head_commit\\.author\\.name",
"github\\.event\\.workflow_run\\.head_commit\\.committer\\.email",
"github\\.event\\.workflow_run\\.head_commit\\.committer\\.name",
]
|
Utils::normalizeExpr(context).regexpMatch(reg)
Utils::normalizeExpr(context).regexpMatch(Utils::wrapRegexp(reg))
)
}

59
ql/test/query-tests/Security/CWE-094/.github/workflows/json_wrap.yml vendored Normal file
View File

@@ -0,0 +1,59 @@
name: Issue Comment Created
on:
issue_comment:
types:
- created
jobs:
jira:
runs-on: ubuntu-latest
if: ${{ github.event.comment.body == '/jira ticket' }}
steps:
- run: echo ${{ github.event.comment.body }}
- name: Login
uses: atlassian/gajira-login@v3
env:
JIRA_BASE_URL: ${{ secrets.JIRA_BASE_URL }}
JIRA_USER_EMAIL: ${{ secrets.JIRA_USER_EMAIL }}
JIRA_API_TOKEN: ${{ secrets.JIRA_API_TOKEN }}
- name: SearchParam
run: echo 'summary ~ ${{ toJSON(github.event.issue.title)}} AND project=${{ secrets.JIRA_PROJECT }}'
- name: Search
id: search
uses: tomhjp/gh-action-jira-search@v0.2.1
with:
jql: 'summary ~ ${{ toJSON(github.event.issue.title)}} AND project=${{ secrets.JIRA_PROJECT }}'
- name: Log
run: echo "Found issue ${{ steps.search.outputs.issue }}"
- name: Create
id: create
if: steps.search.outputs.issue == ''
uses: atlassian/gajira-create@v3
with:
project: ${{ secrets.JIRA_PROJECT }}
issuetype: Task
summary: '${{ github.event.repository.name }}: ${{ github.event.issue.title }}'
description: |
*Issue Link:* ${{ github.event.issue.html_url }}
${{ github.event.issue.body }}
fields: '{"customfield_10006": ${{ toJSON(secrets.JIRA_EPIC_TICKET) }}, "customfield_17401":{"value":${{ toJSON( secrets.JIRA_LAYER_CAKE )}}}}'
- name: Add Comment
if: steps.search.outputs.issue == '' && steps.create.outputs.issue != ''
uses: actions/github-script@v6
with:
github-token: ${{secrets.GITHUB_TOKEN}}
script: |
github.rest.issues.createComment({
issue_number: context.issue.number,
owner: context.repo.owner,
repo: context.repo.repo,
body: '👋 Thanks, Jira [${{steps.create.outputs.issue}}] ticket created.'
})

4
ql/test/query-tests/Security/CWE-094/CodeInjection.expected
View File

@@ -131,6 +131,8 @@ nodes
| .github/workflows/issues.yaml:17:19:17:36 | env.job_env | semmle.label | env.job_env |
| .github/workflows/issues.yaml:18:19:18:37 | env.step_env | semmle.label | env.step_env |
| .github/workflows/issues.yaml:20:20:20:50 | github.event.issue.title | semmle.label | github.event.issue.title |
| .github/workflows/json_wrap.yml:13:20:13:51 | github.event.comment.body | semmle.label | github.event.comment.body |
| .github/workflows/json_wrap.yml:23:31:23:68 | toJSON(github.event.issue.title) | semmle.label | toJSON(github.event.issue.title) |
| .github/workflows/pull_request_review.yml:7:19:7:56 | github.event.pull_request.title | semmle.label | github.event.pull_request.title |
| .github/workflows/pull_request_review.yml:8:19:8:55 | github.event.pull_request.body | semmle.label | github.event.pull_request.body |
| .github/workflows/pull_request_review.yml:9:19:9:61 | github.event.pull_request.head.label | semmle.label | github.event.pull_request.head.label |
@@ -234,6 +236,8 @@ subpaths
| .github/workflows/issues.yaml:15:19:15:39 | env.global_env | .github/workflows/issues.yaml:4:16:4:46 | github.event.issue.title | .github/workflows/issues.yaml:15:19:15:39 | env.global_env | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/issues.yaml:15:19:15:39 | env.global_env | ${{ env.global_env }} |
| .github/workflows/issues.yaml:17:19:17:36 | env.job_env | .github/workflows/issues.yaml:10:17:10:47 | github.event.issue.title | .github/workflows/issues.yaml:17:19:17:36 | env.job_env | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/issues.yaml:17:19:17:36 | env.job_env | ${{ env.job_env }} |
| .github/workflows/issues.yaml:18:19:18:37 | env.step_env | .github/workflows/issues.yaml:20:20:20:50 | github.event.issue.title | .github/workflows/issues.yaml:18:19:18:37 | env.step_env | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/issues.yaml:18:19:18:37 | env.step_env | ${{ env.step_env }} |
| .github/workflows/json_wrap.yml:13:20:13:51 | github.event.comment.body | .github/workflows/json_wrap.yml:13:20:13:51 | github.event.comment.body | .github/workflows/json_wrap.yml:13:20:13:51 | github.event.comment.body | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/json_wrap.yml:13:20:13:51 | github.event.comment.body | ${{ github.event.comment.body }} |
| .github/workflows/json_wrap.yml:23:31:23:68 | toJSON(github.event.issue.title) | .github/workflows/json_wrap.yml:23:31:23:68 | toJSON(github.event.issue.title) | .github/workflows/json_wrap.yml:23:31:23:68 | toJSON(github.event.issue.title) | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/json_wrap.yml:23:31:23:68 | toJSON(github.event.issue.title) | ${{ toJSON(github.event.issue.title)}} |
| .github/workflows/pull_request_review.yml:7:19:7:56 | github.event.pull_request.title | .github/workflows/pull_request_review.yml:7:19:7:56 | github.event.pull_request.title | .github/workflows/pull_request_review.yml:7:19:7:56 | github.event.pull_request.title | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/pull_request_review.yml:7:19:7:56 | github.event.pull_request.title | ${{ github.event.pull_request.title }} |
| .github/workflows/pull_request_review.yml:8:19:8:55 | github.event.pull_request.body | .github/workflows/pull_request_review.yml:8:19:8:55 | github.event.pull_request.body | .github/workflows/pull_request_review.yml:8:19:8:55 | github.event.pull_request.body | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/pull_request_review.yml:8:19:8:55 | github.event.pull_request.body | ${{ github.event.pull_request.body }} |
| .github/workflows/pull_request_review.yml:9:19:9:61 | github.event.pull_request.head.label | .github/workflows/pull_request_review.yml:9:19:9:61 | github.event.pull_request.head.label | .github/workflows/pull_request_review.yml:9:19:9:61 | github.event.pull_request.head.label | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/pull_request_review.yml:9:19:9:61 | github.event.pull_request.head.label | ${{ github.event.pull_request.head.label }} |

4
ql/test/query-tests/Security/CWE-094/PrivilegedCodeInjection.expected
View File

@@ -131,6 +131,8 @@ nodes
| .github/workflows/issues.yaml:17:19:17:36 | env.job_env | semmle.label | env.job_env |
| .github/workflows/issues.yaml:18:19:18:37 | env.step_env | semmle.label | env.step_env |
| .github/workflows/issues.yaml:20:20:20:50 | github.event.issue.title | semmle.label | github.event.issue.title |
| .github/workflows/json_wrap.yml:13:20:13:51 | github.event.comment.body | semmle.label | github.event.comment.body |
| .github/workflows/json_wrap.yml:23:31:23:68 | toJSON(github.event.issue.title) | semmle.label | toJSON(github.event.issue.title) |
| .github/workflows/pull_request_review.yml:7:19:7:56 | github.event.pull_request.title | semmle.label | github.event.pull_request.title |
| .github/workflows/pull_request_review.yml:8:19:8:55 | github.event.pull_request.body | semmle.label | github.event.pull_request.body |
| .github/workflows/pull_request_review.yml:9:19:9:61 | github.event.pull_request.head.label | semmle.label | github.event.pull_request.head.label |
@@ -229,6 +231,8 @@ subpaths
| .github/workflows/issues.yaml:15:19:15:39 | env.global_env | .github/workflows/issues.yaml:4:16:4:46 | github.event.issue.title | .github/workflows/issues.yaml:15:19:15:39 | env.global_env | Potential privileged code injection in $@, which may be controlled by an external user. | .github/workflows/issues.yaml:15:19:15:39 | env.global_env | ${{ env.global_env }} |
| .github/workflows/issues.yaml:17:19:17:36 | env.job_env | .github/workflows/issues.yaml:10:17:10:47 | github.event.issue.title | .github/workflows/issues.yaml:17:19:17:36 | env.job_env | Potential privileged code injection in $@, which may be controlled by an external user. | .github/workflows/issues.yaml:17:19:17:36 | env.job_env | ${{ env.job_env }} |
| .github/workflows/issues.yaml:18:19:18:37 | env.step_env | .github/workflows/issues.yaml:20:20:20:50 | github.event.issue.title | .github/workflows/issues.yaml:18:19:18:37 | env.step_env | Potential privileged code injection in $@, which may be controlled by an external user. | .github/workflows/issues.yaml:18:19:18:37 | env.step_env | ${{ env.step_env }} |
| .github/workflows/json_wrap.yml:13:20:13:51 | github.event.comment.body | .github/workflows/json_wrap.yml:13:20:13:51 | github.event.comment.body | .github/workflows/json_wrap.yml:13:20:13:51 | github.event.comment.body | Potential privileged code injection in $@, which may be controlled by an external user. | .github/workflows/json_wrap.yml:13:20:13:51 | github.event.comment.body | ${{ github.event.comment.body }} |
| .github/workflows/json_wrap.yml:23:31:23:68 | toJSON(github.event.issue.title) | .github/workflows/json_wrap.yml:23:31:23:68 | toJSON(github.event.issue.title) | .github/workflows/json_wrap.yml:23:31:23:68 | toJSON(github.event.issue.title) | Potential privileged code injection in $@, which may be controlled by an external user. | .github/workflows/json_wrap.yml:23:31:23:68 | toJSON(github.event.issue.title) | ${{ toJSON(github.event.issue.title)}} |
| .github/workflows/pull_request_review.yml:7:19:7:56 | github.event.pull_request.title | .github/workflows/pull_request_review.yml:7:19:7:56 | github.event.pull_request.title | .github/workflows/pull_request_review.yml:7:19:7:56 | github.event.pull_request.title | Potential privileged code injection in $@, which may be controlled by an external user. | .github/workflows/pull_request_review.yml:7:19:7:56 | github.event.pull_request.title | ${{ github.event.pull_request.title }} |
| .github/workflows/pull_request_review.yml:8:19:8:55 | github.event.pull_request.body | .github/workflows/pull_request_review.yml:8:19:8:55 | github.event.pull_request.body | .github/workflows/pull_request_review.yml:8:19:8:55 | github.event.pull_request.body | Potential privileged code injection in $@, which may be controlled by an external user. | .github/workflows/pull_request_review.yml:8:19:8:55 | github.event.pull_request.body | ${{ github.event.pull_request.body }} |
| .github/workflows/pull_request_review.yml:9:19:9:61 | github.event.pull_request.head.label | .github/workflows/pull_request_review.yml:9:19:9:61 | github.event.pull_request.head.label | .github/workflows/pull_request_review.yml:9:19:9:61 | github.event.pull_request.head.label | Potential privileged code injection in $@, which may be controlled by an external user. | .github/workflows/pull_request_review.yml:9:19:9:61 | github.event.pull_request.head.label | ${{ github.event.pull_request.head.label }} |