Skip to content

Commit

Permalink
Add -conservative option
Browse files Browse the repository at this point in the history
Update the CLI with a new `-conservative` option that allows users to
only get warnings about GitHub Workflow Expressions that are known to
be controllable by attackers and therefor higher risk.

This option should be useful for users that want to know only about
high risk expressions as well as researchers that want to run this tool
on many projects and get less overwhelmed.

This list of expressions considered in conservative mode covers all of
the expressions considered by:
- ARGUS: A Framework for Staged Static Taint Analysis of GitHub
  Workflows and Actions
- CycodeLabs/raven "Injectable X" queries.
- Semgrep `run-shell-injection` rule for GitHub Actions.
- Actionlint "Script injection by potentially untrusted inputs" check.
- `githubuniverseworkshops/workflow-script-injection` exercise 1.
- `binarytrails/notes` notes on GitHub Actions.
- checkov rule at `github_actions/common/shell_injection_list.py`.
Except for `github.event.inputs ...` which is only included by Semgrep.

Signed-off-by: Eric Cornelissen <ericornelissen@gmail.com>
  • Loading branch information
ericcornelissen committed Mar 29, 2024
1 parent f7bcaa2 commit 672756a
Show file tree
Hide file tree
Showing 8 changed files with 462 additions and 50 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ go run github.com/ericcornelissen/ades/cmd/ades@latest .
- Report dangerous uses of workflow expressions in [`run:`] directives.
- Report dangerous uses of workflow expressions in [`actions/github-script`] scripts.
- _(Experimental)_ Report dangerous uses of workflow expressions in known vulnerable actions.
- Configurable sensitivity.
- Provides suggested fixes.
- Machine & human readable output formats.

Expand Down
29 changes: 12 additions & 17 deletions analyze.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ package ades

import (
"fmt"
"regexp"
)

type Violation struct {
Expand All @@ -27,11 +26,7 @@ type Violation struct {
RuleId string
}

var (
ghaExpressionRegExp = regexp.MustCompile(`\$\{\{.*?\}\}`)
)

func AnalyzeManifest(manifest *Manifest) []Violation {
func AnalyzeManifest(manifest *Manifest, matcher ExprMatcher) []Violation {
violations := make([]Violation, 0)
if manifest == nil {
return violations
Expand All @@ -41,47 +36,47 @@ func AnalyzeManifest(manifest *Manifest) []Violation {
return violations
}

return analyzeSteps(manifest.Runs.Steps)
return analyzeSteps(manifest.Runs.Steps, matcher)
}

func AnalyzeWorkflow(workflow *Workflow) []Violation {
func AnalyzeWorkflow(workflow *Workflow, matcher ExprMatcher) []Violation {
violations := make([]Violation, 0)
if workflow == nil {
return violations
}

for id, job := range workflow.Jobs {
violations = append(violations, analyzeJob(id, &job)...)
violations = append(violations, analyzeJob(id, &job, matcher)...)
}

return violations
}

func analyzeJob(id string, job *WorkflowJob) []Violation {
func analyzeJob(id string, job *WorkflowJob, matcher ExprMatcher) []Violation {
name := job.Name
if name == "" {
name = id
}

violations := make([]Violation, 0)
for _, violation := range analyzeSteps(job.Steps) {
for _, violation := range analyzeSteps(job.Steps, matcher) {
violation.JobId = name
violations = append(violations, violation)
}

return violations
}

func analyzeSteps(steps []JobStep) []Violation {
func analyzeSteps(steps []JobStep, matcher ExprMatcher) []Violation {
violations := make([]Violation, 0)
for i, step := range steps {
violations = append(violations, analyzeStep(i, &step)...)
violations = append(violations, analyzeStep(i, &step, matcher)...)
}

return violations
}

func analyzeStep(id int, step *JobStep) []Violation {
func analyzeStep(id int, step *JobStep, matcher ExprMatcher) []Violation {
name := step.Name
if step.Name == "" {
name = fmt.Sprintf("#%d", id)
Expand All @@ -106,7 +101,7 @@ func analyzeStep(id int, step *JobStep) []Violation {

violations := make([]Violation, 0)
for _, rule := range rules {
for _, violation := range analyzeString(rule.extractFrom(step)) {
for _, violation := range analyzeString(rule.extractFrom(step), matcher) {
violation.RuleId = rule.id
violation.StepId = name
violations = append(violations, violation)
Expand All @@ -116,9 +111,9 @@ func analyzeStep(id int, step *JobStep) []Violation {
return violations
}

func analyzeString(s string) []Violation {
func analyzeString(s string, matcher ExprMatcher) []Violation {
violations := make([]Violation, 0)
if matches := ghaExpressionRegExp.FindAll([]byte(s), len(s)); matches != nil {
if matches := matcher.FindAll([]byte(s)); matches != nil {
for _, problem := range matches {
violations = append(violations, Violation{
Problem: string(problem),
Expand Down
Loading

0 comments on commit 672756a

Please sign in to comment.