diff --git a/expr_insecure.go b/expr_insecure.go new file mode 100644 index 000000000..3850c21a7 --- /dev/null +++ b/expr_insecure.go @@ -0,0 +1,161 @@ +package actionlint + +import ( + "strings" +) + +type untrustedInputMap map[string]untrustedInputMap + +func (m untrustedInputMap) findChild(name string) (untrustedInputMap, bool) { + if m == nil { + return nil, false + } + if c, ok := m[name]; ok { + return c, true + } + if c, ok := m["*"]; ok { + return c, true + } + return nil, false +} + +// BuiltinUntrustedInputs is list of untrusted inputs. These inputs are detected as untrusted in +// `run:` scripts. See the URL for more details. +// https://securitylab.github.com/research/github-actions-untrusted-input/ +var BuiltinUntrustedInputs = untrustedInputMap{ + "github": { + "event": { + "issue": { + "title": nil, + "body": nil, + }, + "pull_request": { + "title": nil, + "body": nil, + "head": { + "ref": nil, + "label": nil, + "repo": { + "default_branch": nil, + }, + }, + }, + "comment": { + "body": nil, + }, + "review": { + "body": nil, + }, + "pages": { + "*": { + "page_name": nil, + }, + }, + "commits": { + "*": { + "message": nil, + "author": { + "email": nil, + "name": nil, + }, + }, + }, + "head_commit": { + "message": nil, + "author": { + "email": nil, + "name": nil, + }, + }, + }, + "head_ref": nil, + }, +} + +// UntrustedInputChecker is a checker to detect untrusted inputs in an expression syntax tree. +type UntrustedInputChecker struct { + root untrustedInputMap + cur untrustedInputMap + varNode *VariableNode + path []string + errs []*ExprError +} + +// NewUntrustedInputChecker creates new UntrustedInputChecker instance. +func NewUntrustedInputChecker(m untrustedInputMap) *UntrustedInputChecker { + return &UntrustedInputChecker{ + root: m, + cur: nil, + varNode: nil, + path: nil, + } +} + +func (u *UntrustedInputChecker) found() { + err := errorfAtExpr(u.varNode, "%q is possibly untrusted. please avoid using it directly in script by passing it through environment variable. see https://securitylab.github.com/research/github-actions-untrusted-input for more details.", strings.Join(u.path, ".")) + u.errs = append(u.errs, err) + u.done() +} + +func (u *UntrustedInputChecker) done() { + if u.cur == nil { + return + } + u.cur = nil + u.varNode = nil + u.path = u.path[:0] +} + +func (u *UntrustedInputChecker) checkVar(name string) { + m, ok := u.root[name] + if !ok { + u.done() + return + } + + u.path = append(u.path, name) + + if m == nil { + u.found() + return + } + + u.cur = m +} + +func (u *UntrustedInputChecker) checkProp(name string) { + c, ok := u.cur.findChild(name) + if !ok { + u.done() + return + } + + u.path = append(u.path, name) + + if c == nil { + u.found() + return + } + + u.cur = c +} + +// OnNode is a callback which should be called on visiting node. This method assumes to be called +// in depth-first, bottom-up order. +func (u *UntrustedInputChecker) OnNode(n ExprNode) { + switch n := n.(type) { + case *VariableNode: + u.varNode = n + u.checkVar(n.Name) + case *ObjectDerefNode: + u.checkProp(n.Property) + default: + u.done() + } +} + +// Errs returns errors detected by this checker. This method should be called after visiting all +// nodes in a syntax tree. +func (u *UntrustedInputChecker) Errs() []*ExprError { + return u.errs +} diff --git a/expr_sema.go b/expr_sema.go index 25186c0e0..7b3761944 100644 --- a/expr_sema.go +++ b/expr_sema.go @@ -284,15 +284,21 @@ type ExprSemanticsChecker struct { vars map[string]ExprType errs []*ExprError varsCopied bool + untrusted *UntrustedInputChecker } -// NewExprSemanticsChecker creates new ExprSemanticsChecker instance. -func NewExprSemanticsChecker() *ExprSemanticsChecker { - return &ExprSemanticsChecker{ +// NewExprSemanticsChecker creates new ExprSemanticsChecker instance. When checkUntrustedInput is +// set to true, the checker will make use of possibly untrusted inputs error. +func NewExprSemanticsChecker(checkUntrustedInput bool) *ExprSemanticsChecker { + c := &ExprSemanticsChecker{ funcs: BuiltinFuncSignatures, vars: BuiltinGlobalVariableTypes, varsCopied: false, } + if checkUntrustedInput { + c.untrusted = NewUntrustedInputChecker(BuiltinUntrustedInputs) + } + return c } func errorAtExpr(e ExprNode, msg string) *ExprError { @@ -346,6 +352,12 @@ func (sema *ExprSemanticsChecker) UpdateNeeds(ty *ObjectType) { sema.vars["needs"] = ty } +func (sema *ExprSemanticsChecker) checkUntrustedInput(n ExprNode) { + if sema.untrusted != nil { + sema.untrusted.OnNode(n) + } +} + func (sema *ExprSemanticsChecker) checkVariable(n *VariableNode) ExprType { v, ok := sema.vars[n.Name] if !ok { @@ -606,6 +618,8 @@ func (sema *ExprSemanticsChecker) checkLogicalOp(n *LogicalOpNode) ExprType { } func (sema *ExprSemanticsChecker) check(expr ExprNode) ExprType { + defer sema.checkUntrustedInput(expr) // Call this method in bottom-up order + switch e := expr.(type) { case *VariableNode: return sema.checkVariable(e) @@ -642,5 +656,9 @@ func (sema *ExprSemanticsChecker) check(expr ExprNode) ExprType { func (sema *ExprSemanticsChecker) Check(expr ExprNode) (ExprType, []*ExprError) { sema.errs = []*ExprError{} ty := sema.check(expr) - return ty, sema.errs + errs := sema.errs + if sema.untrusted != nil { + errs = append(errs, sema.untrusted.Errs()...) + } + return ty, errs } diff --git a/rule_expression.go b/rule_expression.go index 986cfd7cb..8a3e2f304 100644 --- a/rule_expression.go +++ b/rule_expression.go @@ -162,7 +162,7 @@ func (rule *RuleExpression) VisitStep(n *Step) error { var spec *String switch e := n.Exec.(type) { case *ExecRun: - rule.checkString(e.Run) + rule.checkScriptString(e.Run) rule.checkString(e.Shell) rule.checkString(e.WorkingDirectory) case *ExecAction: @@ -422,7 +422,7 @@ func (rule *RuleExpression) checkIfCondition(str *String) { return } - condTy = rule.checkSemanticsOfExprNode(expr, line, col) + condTy = rule.checkSemanticsOfExprNode(expr, line, col, false) } if condTy != nil && !(BoolType{}).Assignable(condTy) { @@ -434,7 +434,14 @@ func (rule *RuleExpression) checkString(str *String) []ExprType { if str == nil { return nil } - return rule.checkExprsIn(str.Value, str.Pos, str.Quoted) + return rule.checkExprsIn(str.Value, str.Pos, str.Quoted, false) +} + +func (rule *RuleExpression) checkScriptString(str *String) []ExprType { + if str == nil { + return nil + } + return rule.checkExprsIn(str.Value, str.Pos, str.Quoted, true) } func (rule *RuleExpression) checkBool(b *Bool) { @@ -464,7 +471,7 @@ func (rule *RuleExpression) checkFloat(f *Float) { rule.checkNumberExpression(f.Expression, "float number value") } -func (rule *RuleExpression) checkExprsIn(s string, pos *Pos, quoted bool) []ExprType { +func (rule *RuleExpression) checkExprsIn(s string, pos *Pos, quoted bool, checkUntrusted bool) []ExprType { // TODO: Line number is not correct when the string contains newlines. line, col := pos.Line, pos.Col @@ -483,7 +490,7 @@ func (rule *RuleExpression) checkExprsIn(s string, pos *Pos, quoted bool) []Expr s = s[start:] offset += start - ty, offsetAfter := rule.checkSemantics(s, line, col+offset) + ty, offsetAfter := rule.checkSemantics(s, line, col+offset, checkUntrusted) if ty == nil || offsetAfter == 0 { return nil } @@ -507,7 +514,7 @@ func (rule *RuleExpression) checkRawYAMLValue(v RawYAMLValue) { rule.checkRawYAMLValue(v) } case *RawYAMLString: - rule.checkExprsIn(v.Value, v.Pos(), false) + rule.checkExprsIn(v.Value, v.Pos(), false, false) default: panic("unreachable") } @@ -518,8 +525,8 @@ func (rule *RuleExpression) exprError(err *ExprError, lineBase, colBase int) { rule.error(pos, err.Message) } -func (rule *RuleExpression) checkSemanticsOfExprNode(expr ExprNode, line, col int) ExprType { - c := NewExprSemanticsChecker() +func (rule *RuleExpression) checkSemanticsOfExprNode(expr ExprNode, line, col int, checkUntrusted bool) ExprType { + c := NewExprSemanticsChecker(checkUntrusted) if rule.matrixTy != nil { c.UpdateMatrix(rule.matrixTy) } @@ -538,7 +545,7 @@ func (rule *RuleExpression) checkSemanticsOfExprNode(expr ExprNode, line, col in return ty } -func (rule *RuleExpression) checkSemantics(src string, line, col int) (ExprType, int) { +func (rule *RuleExpression) checkSemantics(src string, line, col int, checkUntrusted bool) (ExprType, int) { l := NewExprLexer(src) p := NewExprParser() expr, err := p.Parse(l) @@ -546,7 +553,7 @@ func (rule *RuleExpression) checkSemantics(src string, line, col int) (ExprType, rule.exprError(err, line, col) return nil, l.Offset() } - return rule.checkSemanticsOfExprNode(expr, line, col), l.Offset() + return rule.checkSemanticsOfExprNode(expr, line, col, checkUntrusted), l.Offset() } func (rule *RuleExpression) calcNeedsType(job *Job) *ObjectType {