Skip to content

Commit

Permalink
detect workflow_run as a dangerous trigger
Browse files Browse the repository at this point in the history
  • Loading branch information
noamd-legit authored and naveensrinivasan committed Apr 6, 2022
1 parent 606f28a commit 5860896
Show file tree
Hide file tree
Showing 5 changed files with 92 additions and 25 deletions.
57 changes: 36 additions & 21 deletions checks/dangerous_workflow.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,11 @@ const (
type triggerName string

var (
triggerPullRequestTarget = triggerName("pull_request_target")
triggerPullRequest = triggerName("pull_request")
checkoutUntrustedRef = "github.event.pull_request"
triggerPullRequestTarget = triggerName("pull_request_target")
triggerWorkflowRun = triggerName("workflow_run")
triggerPullRequest = triggerName("pull_request")
checkoutUntrustedPullRequestRef = "github.event.pull_request"
checkoutUntrustedWorkflowRunRef = "github.event.workflow_run"
)

// Holds stateful data to pass thru callbacks.
Expand Down Expand Up @@ -168,7 +170,9 @@ func validateSecretsInPullRequests(workflow *actionlint.Workflow, path string,
// We need pull request trigger.
usesPullRequest := usesEventTrigger(workflow, triggerPullRequest)
usesPullRequestTarget := usesEventTrigger(workflow, triggerPullRequestTarget)
if !usesPullRequest && !usesPullRequestTarget {
usesWorkflowRun := usesEventTrigger(workflow, triggerWorkflowRun)

if !usesPullRequest && !usesPullRequestTarget && !usesWorkflowRun {
return nil
}

Expand All @@ -179,6 +183,9 @@ func validateSecretsInPullRequests(workflow *actionlint.Workflow, path string,
if usesPullRequestTarget {
triggers[triggerPullRequestTarget] = usesPullRequestTarget
}
if usesWorkflowRun {
triggers[triggerWorkflowRun] = usesWorkflowRun
}

// Secrets used in env at the top of the wokflow.
if err := checkWorkflowSecretInEnv(workflow, triggers, path, dl, pdata); err != nil {
Expand All @@ -198,7 +205,7 @@ func validateSecretsInPullRequests(workflow *actionlint.Workflow, path string,
func validateUntrustedCodeCheckout(workflow *actionlint.Workflow, path string,
dl checker.DetailLogger, pdata *patternCbData,
) error {
if !usesEventTrigger(workflow, triggerPullRequestTarget) {
if !usesEventTrigger(workflow, triggerPullRequestTarget) && !usesEventTrigger(workflow, triggerWorkflowRun) {
return nil
}

Expand Down Expand Up @@ -240,16 +247,7 @@ func checkJobForUsedSecrets(job *actionlint.Job, triggers map[triggerName]bool,

// If the job has an environment, assume it's an env secret gated by
// some approval and don't alert.
if jobUsesEnvironment(job) {
return nil
}

// For pull request target, we need a ref to the pull request.
_, usesPullRequest := triggers[triggerPullRequest]
_, usesPullRequestTarget := triggers[triggerPullRequestTarget]
chk, ref := jobUsesCodeCheckout(job)
if !((chk && usesPullRequest) ||
(chk && usesPullRequestTarget && strings.Contains(ref, checkoutUntrustedRef))) {
if !jobUsesCodeCheckoutAndNoEnvironment(job, triggers) {
return nil
}

Expand Down Expand Up @@ -281,17 +279,32 @@ func workflowUsesCodeCheckoutAndNoEnvironment(workflow *actionlint.Workflow,
return false
}

for _, job := range workflow.Jobs {
if jobUsesCodeCheckoutAndNoEnvironment(job, triggers) {
return true
}
}
return false
}

func jobUsesCodeCheckoutAndNoEnvironment(job *actionlint.Job, triggers map[triggerName]bool,
) bool {
if job == nil {
return false
}
_, usesPullRequest := triggers[triggerPullRequest]
_, usesPullRequestTarget := triggers[triggerPullRequestTarget]
_, usesWorkflowRun := triggers[triggerWorkflowRun]

for _, job := range workflow.Jobs {
chk, ref := jobUsesCodeCheckout(job)
if ((chk && usesPullRequest) ||
(chk && usesPullRequestTarget && strings.Contains(ref, checkoutUntrustedRef))) &&
!jobUsesEnvironment(job) {
chk, ref := jobUsesCodeCheckout(job)
if !jobUsesEnvironment(job) {
if (chk && usesPullRequest) ||
(chk && usesPullRequestTarget && strings.Contains(ref, checkoutUntrustedPullRequestRef)) ||
(chk && usesWorkflowRun && strings.Contains(ref, checkoutUntrustedWorkflowRunRef)) {
return true
}
}

return false
}

Expand Down Expand Up @@ -348,7 +361,9 @@ func checkJobForUntrustedCodeCheckout(job *actionlint.Job, path string,
if !ok || ref.Value == nil {
continue
}
if strings.Contains(ref.Value.Value, checkoutUntrustedRef) {

if strings.Contains(ref.Value.Value, checkoutUntrustedPullRequestRef) ||
strings.Contains(ref.Value.Value, checkoutUntrustedWorkflowRunRef) {
line := fileparser.GetLineNumber(step.Pos)
dl.Warn(&checker.LogMessage{
Path: path,
Expand Down
11 changes: 11 additions & 0 deletions checks/dangerous_workflow_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,17 @@ func TestGithubDangerousWorkflow(t *testing.T) {
NumberOfDebug: 0,
},
},
{
name: "run untrusted code checkout test - workflow_run",
filename: "./testdata/.github/workflows/github-workflow-dangerous-pattern-untrusted-checkout-workflow_run.yml",
expected: scut.TestReturn{
Error: nil,
Score: checker.MinResultScore,
NumberOfWarn: 2,
NumberOfInfo: 0,
NumberOfDebug: 0,
},
},
{
name: "run untrusted code checkout test",
filename: "./testdata/.github/workflows/github-workflow-dangerous-pattern-untrusted-checkout.yml",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# Copyright 2021 Security Scorecard Authors
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
on:
workflow_run:
workflows: ['dummy']

jobs:
build:
name: Build and test
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
with:
ref: ${{ github.event.workflow_run }}

- uses: actions/setup-node@v1
- run: |
npm install
npm build
- uses: completely/fakeaction@v2
with:
arg1: ${{ secrets.supersecret }}

- uses: fakerepo/comment-on-pr@v1
with:
message: |
Thank you!
6 changes: 4 additions & 2 deletions docs/checks.md
Original file line number Diff line number Diff line change
Expand Up @@ -264,8 +264,8 @@ logging github context and secrets, or use of potentially untrusted inputs in sc
The following patterns are checked:

Untrusted Code Checkout: This is the misuse of potentially dangerous triggers.
This checks if a `pull_request_target` workflow trigger was used in conjunction
with an explicit pull request checkout. Workflows triggered with `pull_request_target`
This checks if a `pull_request_target` or `workflow_run` workflow trigger was used in conjunction
with an explicit pull request checkout. Workflows triggered with `pull_request_target` / `workflow_run`
have write permission to the target repository and access to target repository
secrets. With the PR checkout, PR authors may compromise the repository, for
example, by using build scripts controlled by the author of the PR or reading
Expand Down Expand Up @@ -606,8 +606,10 @@ possible.
Risk: `Critical` (service possibly accessible to third parties)

This check determines whether the webhook defined in the repository has a token configured to authenticate the origins of requests.


**Remediation steps**
- Check whether your service supports token authentication.
- If there is support for token authentication, set the secret in the webhook configuration. See [Setting up a webhook](https://docs.github.com/en/developers/webhooks-and-events/webhooks/creating-webhooks#setting-up-a-webhook)
- If there is no support for token authentication, consider implementing it by following [these directions](https://docs.github.com/en/developers/webhooks-and-events/webhooks/securing-your-webhooks).

4 changes: 2 additions & 2 deletions docs/checks/internal/checks.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -674,8 +674,8 @@ checks:
The following patterns are checked:
Untrusted Code Checkout: This is the misuse of potentially dangerous triggers.
This checks if a `pull_request_target` workflow trigger was used in conjunction
with an explicit pull request checkout. Workflows triggered with `pull_request_target`
This checks if a `pull_request_target` or `workflow_run` workflow trigger was used in conjunction
with an explicit pull request checkout. Workflows triggered with `pull_request_target` / `workflow_run`
have write permission to the target repository and access to target repository
secrets. With the PR checkout, PR authors may compromise the repository, for
example, by using build scripts controlled by the author of the PR or reading
Expand Down

0 comments on commit 5860896

Please sign in to comment.