Dangerous Workflow: some user input are not being detected as untrusted input.

[diogoteles08]

Is your feature request related to a problem? Please describe. The current Dangerous Workflow check looks for places where untrusted user inputs could be used for Script Injection, such as:

- name: Check title
  run: |
    title="${{ github.event.issue.title }}"
    if [[ ! $title =~ ^.*:\ .*$ ]]; then
      echo "Bad issue title"
      exit 1
    fi

The problem is that Scorecard is not considering the whole range of user input that might lead to Script Injection. The piece of code that lists the user inputs is the following:

func containsUntrustedContextPattern(variable string) bool {
        // GitHub event context details that may be attacker controlled.
        // See https://securitylab.github.com/research/github-actions-untrusted-input/
        untrustedContextPattern := regexp.MustCompile(
                `.*(issue\.title|` +
                        `issue\.body|` +
                        `pull_request\.title|` +
                        `pull_request\.body|` +
                        `comment\.body|` +
                        `review\.body|` +
                        `review_comment\.body|` +
                        `pages.*\.page_name|` +
                        `commits.*\.message|` +
                        `head_commit\.message|` +
                        `head_commit\.author\.email|` +
                        `head_commit\.author\.name|` +
                        `commits.*\.author\.email|` +
                        `commits.*\.author\.name|` +
                        `pull_request\.head\.ref|` +
                        `pull_request\.head\.label|` +
                        `pull_request\.head\.repo\.default_branch).*`)

        if strings.Contains(variable, "github.head_ref") {
                return true
        }
        return strings.Contains(variable, "github.event.") && untrustedContextPattern.MatchString(variable)
}

And I can point that it isn't able to detect the following inputs, for example:

• ${{ github.event.issue_comment.comment.body }}, described in github's doc
• ${{ github.event.commit_comment.comment.body }}, described in github's doc
• ${{ github.event.fork.forkee.name }}

Describe the solution you'd like As Dangerous Workflow is a very important check, Scorecard should be able to identify as many Script Injection risks as possible.

We should study more types of GitHub Events, their payloads, and make Scorecard as reliable as possible to identify different ways that Script Injections could take place.

Additional context I'd be happy to complete my search and also raise a PR to fix this, but I won't be able to do this in a near future. However, I'm marking this as "Good First issue", as the biggest effort to solve this would be actually a research on GitHub's API. The changes on Scorecard would be simple.

[spencerschrock]

And I can point that it isn't able to detect the following inputs, for example:

• ${{ github.event.issue_comment.comment.body }}, described in github's doc
• ${{ github.event.commit_comment.comment.body }}, described in github's doc

We detect the first two at least: https://go.dev/play/p/Saz4FZ7j2A-

But yeah, there are probably more to consider. #3554, #2236, etc.

[diogoteles08]

Thanks Spencer! I failed on the regex mental compilation >< @pnacht also pointed out that ${{ github.event.fork.forkee.name }} shouldn't be considered dangerous, because the rules for repo names make it impossible to craft a malicious name (no spaces, no slashes, etc).

But I've found the following ones that are probably dangerous and not covered by scorecard:

• "${{ github.event.discussion.body }}"
• "${{ github.event.discussion.title }}"

[pnacht]

Another set of potentially risky variables is github.event.{head_commit/commits[*]}.committer.{name/email}. Scorecard detects the data for the author, but not the committer.