mirror of
https://github.com/securego/gosec.git
synced 2024-12-24 11:35:52 +00:00
Redesign and reimplement the slice out of bounds check using SSA code representation
Signed-off-by: Cosmin Cojocar <gcojocar@adobe.com>
This commit is contained in:
parent
e1278f9572
commit
e02e2f6d5b
8 changed files with 461 additions and 510 deletions
|
@ -1,7 +1,7 @@
|
|||
|
||||
# gosec - Golang Security Checker
|
||||
|
||||
Inspects source code for security problems by scanning the Go AST.
|
||||
Inspects source code for security problems by scanning the Go AST and SSA code representation.
|
||||
|
||||
<img src="https://securego.io/img/gosec.png" width="320">
|
||||
|
||||
|
|
10
analyzer.go
10
analyzer.go
|
@ -231,9 +231,7 @@ func (gosec *Analyzer) Process(buildTags []string, packagePaths ...string) error
|
|||
return fmt.Errorf("parsing errors in pkg %q: %w", pkg.Name, err)
|
||||
}
|
||||
gosec.CheckRules(pkg)
|
||||
if on, err := gosec.config.IsGlobalEnabled(SSA); err == nil && on {
|
||||
gosec.CheckAnalyzers(pkg)
|
||||
}
|
||||
gosec.CheckAnalyzers(pkg)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -377,8 +375,10 @@ func (gosec *Analyzer) CheckAnalyzers(pkg *packages.Package) {
|
|||
continue
|
||||
}
|
||||
if result != nil {
|
||||
if aissue, ok := result.(*issue.Issue); ok {
|
||||
gosec.updateIssues(aissue, false, []issue.SuppressionInfo{})
|
||||
if passIssues, ok := result.([]*issue.Issue); ok {
|
||||
for _, iss := range passIssues {
|
||||
gosec.updateIssues(iss, false, []issue.SuppressionInfo{})
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
376
analyzers/slice_bounds.go
Normal file
376
analyzers/slice_bounds.go
Normal file
|
@ -0,0 +1,376 @@
|
|||
// (c) Copyright gosec's 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.
|
||||
|
||||
package analyzers
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"go/token"
|
||||
"regexp"
|
||||
"strconv"
|
||||
"strings"
|
||||
|
||||
"golang.org/x/tools/go/analysis"
|
||||
"golang.org/x/tools/go/analysis/passes/buildssa"
|
||||
"golang.org/x/tools/go/ssa"
|
||||
|
||||
"github.com/securego/gosec/v2/issue"
|
||||
)
|
||||
|
||||
type bound int
|
||||
|
||||
const (
|
||||
lowerUnbounded bound = iota
|
||||
upperUnbounded
|
||||
unbounded
|
||||
upperBounded
|
||||
)
|
||||
|
||||
func newSliceBoundsAnalyzer(id string, description string) *analysis.Analyzer {
|
||||
return &analysis.Analyzer{
|
||||
Name: id,
|
||||
Doc: description,
|
||||
Run: runSliceBounds,
|
||||
Requires: []*analysis.Analyzer{buildssa.Analyzer},
|
||||
}
|
||||
}
|
||||
|
||||
func runSliceBounds(pass *analysis.Pass) (interface{}, error) {
|
||||
ssaResult, err := getSSAResult(pass)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
issues := map[ssa.Instruction]*issue.Issue{}
|
||||
ifs := map[ssa.If]*ssa.BinOp{}
|
||||
for _, mcall := range ssaResult.SSA.SrcFuncs {
|
||||
for _, block := range mcall.DomPreorder() {
|
||||
for _, instr := range block.Instrs {
|
||||
switch instr := instr.(type) {
|
||||
case *ssa.Alloc:
|
||||
cap, err := extractSliceCapFromAlloc(instr.String())
|
||||
if err != nil {
|
||||
break
|
||||
}
|
||||
allocRefs := instr.Referrers()
|
||||
if allocRefs == nil {
|
||||
break
|
||||
}
|
||||
for _, instr := range *allocRefs {
|
||||
if slice, ok := instr.(*ssa.Slice); ok {
|
||||
if _, ok := slice.X.(*ssa.Alloc); ok {
|
||||
if slice.Parent() != nil {
|
||||
l, h := extractSliceBounds(slice)
|
||||
newCap := computeSliceNewCap(l, h, cap)
|
||||
violations := []ssa.Instruction{}
|
||||
trackSliceBounds(newCap, slice, &violations, ifs)
|
||||
for _, s := range violations {
|
||||
switch s := s.(type) {
|
||||
case *ssa.Slice:
|
||||
issue := newIssue(
|
||||
pass.Analyzer.Name,
|
||||
"slice bounds out of range",
|
||||
pass.Fset,
|
||||
s.Pos(),
|
||||
issue.Low,
|
||||
issue.High)
|
||||
issues[s] = issue
|
||||
case *ssa.IndexAddr:
|
||||
issue := newIssue(
|
||||
pass.Analyzer.Name,
|
||||
"slice index out of range",
|
||||
pass.Fset,
|
||||
s.Pos(),
|
||||
issue.Low,
|
||||
issue.High)
|
||||
issues[s] = issue
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
for ifref, binop := range ifs {
|
||||
bound, value, err := extractBinOpBound(binop)
|
||||
if err != nil {
|
||||
continue
|
||||
}
|
||||
for i, block := range ifref.Block().Succs {
|
||||
if i == 1 {
|
||||
bound = invBound(bound)
|
||||
}
|
||||
for _, instr := range block.Instrs {
|
||||
if _, ok := issues[instr]; ok {
|
||||
switch bound {
|
||||
case lowerUnbounded:
|
||||
break
|
||||
case upperUnbounded, unbounded:
|
||||
delete(issues, instr)
|
||||
case upperBounded:
|
||||
switch tinstr := instr.(type) {
|
||||
case *ssa.Slice:
|
||||
lower, upper := extractSliceBounds(tinstr)
|
||||
if isSliceInsideBounds(0, value, lower, upper) {
|
||||
delete(issues, instr)
|
||||
}
|
||||
case *ssa.IndexAddr:
|
||||
indexValue, err := extractIntValue(tinstr.Index.String())
|
||||
if err != nil {
|
||||
break
|
||||
}
|
||||
if isSliceIndexInsideBounds(0, value, indexValue) {
|
||||
delete(issues, instr)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
foundIssues := []*issue.Issue{}
|
||||
for _, issue := range issues {
|
||||
foundIssues = append(foundIssues, issue)
|
||||
}
|
||||
if len(foundIssues) > 0 {
|
||||
return foundIssues, nil
|
||||
}
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
func trackSliceBounds(cap int, slice ssa.Node, violations *[]ssa.Instruction, ifs map[ssa.If]*ssa.BinOp) {
|
||||
if violations == nil {
|
||||
violations = &[]ssa.Instruction{}
|
||||
}
|
||||
referrers := slice.Referrers()
|
||||
if referrers != nil {
|
||||
for _, refinstr := range *referrers {
|
||||
switch refinstr := refinstr.(type) {
|
||||
case *ssa.Slice:
|
||||
checkAllSlicesBounds(cap, refinstr, violations, ifs)
|
||||
switch refinstr.X.(type) {
|
||||
case *ssa.Alloc, *ssa.Parameter:
|
||||
l, h := extractSliceBounds(refinstr)
|
||||
newCap := computeSliceNewCap(l, h, cap)
|
||||
trackSliceBounds(newCap, refinstr, violations, ifs)
|
||||
}
|
||||
case *ssa.IndexAddr:
|
||||
indexValue, err := extractIntValue(refinstr.Index.String())
|
||||
if err == nil && !isSliceIndexInsideBounds(0, cap, indexValue) {
|
||||
*violations = append(*violations, refinstr)
|
||||
}
|
||||
case *ssa.Call:
|
||||
if ifref, cond := extractSliceIfLenCondition(refinstr); ifref != nil && cond != nil {
|
||||
ifs[*ifref] = cond
|
||||
} else {
|
||||
parPos := -1
|
||||
for pos, arg := range refinstr.Call.Args {
|
||||
if a, ok := arg.(*ssa.Slice); ok && a == slice {
|
||||
parPos = pos
|
||||
}
|
||||
}
|
||||
if fn, ok := refinstr.Call.Value.(*ssa.Function); ok {
|
||||
if len(fn.Params) > parPos && parPos > -1 {
|
||||
param := fn.Params[parPos]
|
||||
trackSliceBounds(cap, param, violations, ifs)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func checkAllSlicesBounds(cap int, slice *ssa.Slice, violations *[]ssa.Instruction, ifs map[ssa.If]*ssa.BinOp) {
|
||||
if violations == nil {
|
||||
violations = &[]ssa.Instruction{}
|
||||
}
|
||||
sliceLow, sliceHigh := extractSliceBounds(slice)
|
||||
if !isSliceInsideBounds(0, cap, sliceLow, sliceHigh) {
|
||||
*violations = append(*violations, slice)
|
||||
}
|
||||
switch slice.X.(type) {
|
||||
case *ssa.Alloc, *ssa.Parameter, *ssa.Slice:
|
||||
l, h := extractSliceBounds(slice)
|
||||
newCap := computeSliceNewCap(l, h, cap)
|
||||
trackSliceBounds(newCap, slice, violations, ifs)
|
||||
}
|
||||
|
||||
references := slice.Referrers()
|
||||
if references == nil {
|
||||
return
|
||||
}
|
||||
for _, ref := range *references {
|
||||
switch s := ref.(type) {
|
||||
case *ssa.Slice:
|
||||
checkAllSlicesBounds(cap, s, violations, ifs)
|
||||
switch s.X.(type) {
|
||||
case *ssa.Alloc, *ssa.Parameter:
|
||||
l, h := extractSliceBounds(s)
|
||||
newCap := computeSliceNewCap(l, h, cap)
|
||||
trackSliceBounds(newCap, s, violations, ifs)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func extractSliceIfLenCondition(call *ssa.Call) (*ssa.If, *ssa.BinOp) {
|
||||
if builtInLen, ok := call.Call.Value.(*ssa.Builtin); ok {
|
||||
if builtInLen.Name() == "len" {
|
||||
refs := call.Referrers()
|
||||
if refs != nil {
|
||||
for _, ref := range *refs {
|
||||
if binop, ok := ref.(*ssa.BinOp); ok {
|
||||
binoprefs := binop.Referrers()
|
||||
for _, ref := range *binoprefs {
|
||||
if ifref, ok := ref.(*ssa.If); ok {
|
||||
return ifref, binop
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
func computeSliceNewCap(l, h, cap int) int {
|
||||
if l == 0 && h == 0 {
|
||||
return cap
|
||||
}
|
||||
if l > 0 && h == 0 {
|
||||
return cap - l
|
||||
}
|
||||
if l == 0 && h > 0 {
|
||||
return h
|
||||
}
|
||||
return h - l
|
||||
}
|
||||
|
||||
func invBound(bound bound) bound {
|
||||
switch bound {
|
||||
case lowerUnbounded:
|
||||
return upperUnbounded
|
||||
case upperUnbounded:
|
||||
return lowerUnbounded
|
||||
case upperBounded:
|
||||
return unbounded
|
||||
case unbounded:
|
||||
return upperBounded
|
||||
default:
|
||||
return unbounded
|
||||
}
|
||||
}
|
||||
|
||||
func extractBinOpBound(binop *ssa.BinOp) (bound, int, error) {
|
||||
if binop.X != nil {
|
||||
if x, ok := binop.X.(*ssa.Const); ok {
|
||||
value, err := strconv.Atoi(x.Value.String())
|
||||
if err != nil {
|
||||
return lowerUnbounded, value, err
|
||||
}
|
||||
switch binop.Op {
|
||||
case token.LSS, token.LEQ:
|
||||
return upperUnbounded, value, nil
|
||||
case token.GTR, token.GEQ:
|
||||
return lowerUnbounded, value, nil
|
||||
case token.EQL:
|
||||
return upperBounded, value, nil
|
||||
case token.NEQ:
|
||||
return unbounded, value, nil
|
||||
}
|
||||
}
|
||||
}
|
||||
if binop.Y != nil {
|
||||
if y, ok := binop.Y.(*ssa.Const); ok {
|
||||
value, err := strconv.Atoi(y.Value.String())
|
||||
if err != nil {
|
||||
return lowerUnbounded, value, err
|
||||
}
|
||||
switch binop.Op {
|
||||
case token.LSS, token.LEQ:
|
||||
return lowerUnbounded, value, nil
|
||||
case token.GTR, token.GEQ:
|
||||
return upperUnbounded, value, nil
|
||||
case token.EQL:
|
||||
return upperBounded, value, nil
|
||||
case token.NEQ:
|
||||
return unbounded, value, nil
|
||||
}
|
||||
}
|
||||
}
|
||||
return lowerUnbounded, 0, fmt.Errorf("unable to extract constant from binop")
|
||||
}
|
||||
|
||||
func isSliceIndexInsideBounds(l, h int, index int) bool {
|
||||
return (l <= index && index < h)
|
||||
}
|
||||
|
||||
func isSliceInsideBounds(l, h int, cl, ch int) bool {
|
||||
return (l <= cl && h >= ch) && (l <= ch && h >= cl)
|
||||
}
|
||||
|
||||
func extractSliceBounds(slice *ssa.Slice) (int, int) {
|
||||
var low int
|
||||
if slice.Low != nil {
|
||||
l, err := extractIntValue(slice.Low.String())
|
||||
if err == nil {
|
||||
low = l
|
||||
}
|
||||
}
|
||||
var high int
|
||||
if slice.High != nil {
|
||||
h, err := extractIntValue(slice.High.String())
|
||||
if err == nil {
|
||||
high = h
|
||||
}
|
||||
}
|
||||
return low, high
|
||||
}
|
||||
|
||||
func extractIntValue(value string) (int, error) {
|
||||
parts := strings.Split(value, ":")
|
||||
if len(parts) != 2 {
|
||||
return 0, fmt.Errorf("invalid value: %s", value)
|
||||
}
|
||||
if parts[1] != "int" {
|
||||
return 0, fmt.Errorf("invalid value: %s", value)
|
||||
}
|
||||
return strconv.Atoi(parts[0])
|
||||
}
|
||||
|
||||
func extractSliceCapFromAlloc(instr string) (int, error) {
|
||||
re := regexp.MustCompile(`new \[(\d+)\]*`)
|
||||
var cap int
|
||||
matches := re.FindAllStringSubmatch(instr, -1)
|
||||
if matches == nil {
|
||||
return cap, errors.New("no slice cap found")
|
||||
}
|
||||
|
||||
if len(matches) > 0 {
|
||||
m := matches[0]
|
||||
if len(m) > 1 {
|
||||
return strconv.Atoi(m[1])
|
||||
}
|
||||
}
|
||||
|
||||
return 0, errors.New("no slice cap found")
|
||||
}
|
|
@ -1,57 +0,0 @@
|
|||
// (c) Copyright gosec's 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.
|
||||
|
||||
package analyzers
|
||||
|
||||
import (
|
||||
"golang.org/x/tools/go/analysis"
|
||||
"golang.org/x/tools/go/analysis/passes/buildssa"
|
||||
"golang.org/x/tools/go/ssa"
|
||||
|
||||
"github.com/securego/gosec/v2/issue"
|
||||
)
|
||||
|
||||
func newSSRFAnalyzer(id string, description string) *analysis.Analyzer {
|
||||
return &analysis.Analyzer{
|
||||
Name: id,
|
||||
Doc: description,
|
||||
Run: runSSRF,
|
||||
Requires: []*analysis.Analyzer{buildssa.Analyzer},
|
||||
}
|
||||
}
|
||||
|
||||
func runSSRF(pass *analysis.Pass) (interface{}, error) {
|
||||
ssaResult, err := getSSAResult(pass)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
// TODO: implement the analysis
|
||||
for _, fn := range ssaResult.SSA.SrcFuncs {
|
||||
for _, block := range fn.DomPreorder() {
|
||||
for _, instr := range block.Instrs {
|
||||
switch instr := instr.(type) {
|
||||
case *ssa.Call:
|
||||
callee := instr.Call.StaticCallee()
|
||||
if callee != nil {
|
||||
ssaResult.Logger.Printf("callee: %s\n", callee)
|
||||
return newIssue(pass.Analyzer.Name,
|
||||
"not implemented",
|
||||
pass.Fset, instr.Call.Pos(), issue.Low, issue.High), nil
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
return nil, nil
|
||||
}
|
|
@ -38,7 +38,7 @@ type SSAAnalyzerResult struct {
|
|||
// BuildDefaultAnalyzers returns the default list of analyzers
|
||||
func BuildDefaultAnalyzers() []*analysis.Analyzer {
|
||||
return []*analysis.Analyzer{
|
||||
newSSRFAnalyzer("G107", "URL provided to HTTP request as taint input"),
|
||||
newSliceBoundsAnalyzer("G602", "Possible slice bounds out of range"),
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
@ -107,7 +107,6 @@ func Generate(trackSuppressions bool, filters ...RuleFilter) RuleList {
|
|||
|
||||
// memory safety
|
||||
{"G601", "Implicit memory aliasing in RangeStmt", NewImplicitAliasing},
|
||||
{"G602", "Slice access out of bounds", NewSliceBoundCheck},
|
||||
}
|
||||
|
||||
ruleMap := make(map[string]RuleDefinition)
|
||||
|
|
|
@ -1,410 +0,0 @@
|
|||
package rules
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"go/ast"
|
||||
"go/types"
|
||||
|
||||
"github.com/securego/gosec/v2"
|
||||
"github.com/securego/gosec/v2/issue"
|
||||
)
|
||||
|
||||
// sliceOutOfBounds is a rule which checks for slices which are accessed outside their capacity,
|
||||
// either through indexing it out of bounds or through slice expressions whose low or high index
|
||||
// are out of bounds.
|
||||
type sliceOutOfBounds struct {
|
||||
sliceCaps map[*ast.CallExpr]map[string]*int64 // Capacities of slices. Maps function call -> var name -> value.
|
||||
currentScope *types.Scope // Current scope. Map is cleared when scope changes.
|
||||
currentFuncName string // Current function.
|
||||
funcCallArgs map[string][]*int64 // Caps to load once a func declaration is scanned.
|
||||
issue.MetaData // Metadata for this rule.
|
||||
}
|
||||
|
||||
// ID returns the rule ID for sliceOutOfBounds: G602.
|
||||
func (s *sliceOutOfBounds) ID() string {
|
||||
return s.MetaData.ID
|
||||
}
|
||||
|
||||
func (s *sliceOutOfBounds) Match(node ast.Node, ctx *gosec.Context) (*issue.Issue, error) {
|
||||
if s.currentScope == nil {
|
||||
s.currentScope = ctx.Pkg.Scope()
|
||||
} else if s.currentScope != ctx.Pkg.Scope() {
|
||||
s.currentScope = ctx.Pkg.Scope()
|
||||
|
||||
// Clear slice map, since we are in a new scope
|
||||
sliceMapNil := make(map[string]*int64)
|
||||
sliceCaps := make(map[*ast.CallExpr]map[string]*int64)
|
||||
sliceCaps[nil] = sliceMapNil
|
||||
s.sliceCaps = sliceCaps
|
||||
}
|
||||
|
||||
switch node := node.(type) {
|
||||
case *ast.AssignStmt:
|
||||
return s.matchAssign(node, ctx)
|
||||
case *ast.SliceExpr:
|
||||
return s.matchSliceExpr(node, ctx)
|
||||
case *ast.IndexExpr:
|
||||
return s.matchIndexExpr(node, ctx)
|
||||
case *ast.FuncDecl:
|
||||
s.currentFuncName = node.Name.Name
|
||||
s.loadArgCaps(node)
|
||||
case *ast.CallExpr:
|
||||
if _, ok := node.Fun.(*ast.FuncLit); ok {
|
||||
// Do nothing with func literals for now.
|
||||
break
|
||||
}
|
||||
|
||||
sliceMap := make(map[string]*int64)
|
||||
s.sliceCaps[node] = sliceMap
|
||||
s.setupCallArgCaps(node, ctx)
|
||||
}
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
// updateSliceCaps takes in a variable name and a map of calls we are updating the variables for to the updated values
|
||||
// and will add it to the sliceCaps map.
|
||||
func (s *sliceOutOfBounds) updateSliceCaps(varName string, caps map[*ast.CallExpr]*int64) {
|
||||
for callExpr, cap := range caps {
|
||||
s.sliceCaps[callExpr][varName] = cap
|
||||
}
|
||||
}
|
||||
|
||||
// getAllCalls returns all CallExprs that are calls to the given function.
|
||||
func (s *sliceOutOfBounds) getAllCalls(funcName string, ctx *gosec.Context) []*ast.CallExpr {
|
||||
calls := []*ast.CallExpr{}
|
||||
|
||||
for callExpr := range s.sliceCaps {
|
||||
if callExpr != nil {
|
||||
// Compare the names of the function the code is scanning with the current call we are iterating over
|
||||
_, callFuncName, err := gosec.GetCallInfo(callExpr, ctx)
|
||||
if err != nil {
|
||||
continue
|
||||
}
|
||||
|
||||
if callFuncName == funcName {
|
||||
calls = append(calls, callExpr)
|
||||
}
|
||||
}
|
||||
}
|
||||
return calls
|
||||
}
|
||||
|
||||
// getSliceCapsForFunc gets all the capacities for slice with given name that are stored for each call to the passed function.
|
||||
func (s *sliceOutOfBounds) getSliceCapsForFunc(funcName string, varName string, ctx *gosec.Context) map[*ast.CallExpr]*int64 {
|
||||
caps := make(map[*ast.CallExpr]*int64)
|
||||
|
||||
calls := s.getAllCalls(funcName, ctx)
|
||||
for _, call := range calls {
|
||||
if callCaps, ok := s.sliceCaps[call]; ok {
|
||||
caps[call] = callCaps[varName]
|
||||
}
|
||||
}
|
||||
|
||||
return caps
|
||||
}
|
||||
|
||||
// setupCallArgCaps evaluates and saves the caps for any slices in the args so they can be validated when the function is scanned.
|
||||
func (s *sliceOutOfBounds) setupCallArgCaps(callExpr *ast.CallExpr, ctx *gosec.Context) {
|
||||
// Array of caps to be loaded once the function declaration is scanned
|
||||
funcCallArgs := []*int64{}
|
||||
|
||||
// Get function name
|
||||
_, funcName, err := gosec.GetCallInfo(callExpr, ctx)
|
||||
if err != nil {
|
||||
return
|
||||
}
|
||||
|
||||
for _, arg := range callExpr.Args {
|
||||
switch node := arg.(type) {
|
||||
case *ast.SliceExpr:
|
||||
caps := s.evaluateSliceExpr(node, ctx)
|
||||
|
||||
// Simplifying assumption: use the lowest capacity. Storing all possible capacities for slices passed
|
||||
// to a function call would catch the most issues, but would require a data structure like a stack and a
|
||||
// reworking of the code for scanning itself. Use the lowest capacity, as this would be more likely to
|
||||
// raise an issue for being out of bounds.
|
||||
var lowestCap *int64
|
||||
for _, cap := range caps {
|
||||
if cap == nil {
|
||||
continue
|
||||
}
|
||||
|
||||
if lowestCap == nil {
|
||||
lowestCap = cap
|
||||
} else if *lowestCap > *cap {
|
||||
lowestCap = cap
|
||||
}
|
||||
}
|
||||
|
||||
if lowestCap == nil {
|
||||
funcCallArgs = append(funcCallArgs, nil)
|
||||
continue
|
||||
}
|
||||
|
||||
// Now create a map of just this value to add it to the sliceCaps
|
||||
funcCallArgs = append(funcCallArgs, lowestCap)
|
||||
case *ast.Ident:
|
||||
ident := arg.(*ast.Ident)
|
||||
caps := s.getSliceCapsForFunc(s.currentFuncName, ident.Name, ctx)
|
||||
|
||||
var lowestCap *int64
|
||||
for _, cap := range caps {
|
||||
if cap == nil {
|
||||
continue
|
||||
}
|
||||
|
||||
if lowestCap == nil {
|
||||
lowestCap = cap
|
||||
} else if *lowestCap > *cap {
|
||||
lowestCap = cap
|
||||
}
|
||||
}
|
||||
|
||||
if lowestCap == nil {
|
||||
funcCallArgs = append(funcCallArgs, nil)
|
||||
continue
|
||||
}
|
||||
|
||||
// Now create a map of just this value to add it to the sliceCaps
|
||||
funcCallArgs = append(funcCallArgs, lowestCap)
|
||||
default:
|
||||
funcCallArgs = append(funcCallArgs, nil)
|
||||
}
|
||||
}
|
||||
s.funcCallArgs[funcName] = funcCallArgs
|
||||
}
|
||||
|
||||
// loadArgCaps loads caps that were saved for a call to this function.
|
||||
func (s *sliceOutOfBounds) loadArgCaps(funcDecl *ast.FuncDecl) {
|
||||
sliceMap := make(map[string]*int64)
|
||||
funcName := funcDecl.Name.Name
|
||||
|
||||
// Create a dummmy call expr for the new function. This is so we can still store args for
|
||||
// functions which are not explicitly called in the code by other functions (specifically, main).
|
||||
ident := ast.NewIdent(funcName)
|
||||
dummyCallExpr := ast.CallExpr{
|
||||
Fun: ident,
|
||||
}
|
||||
|
||||
argCaps, ok := s.funcCallArgs[funcName]
|
||||
if !ok || len(argCaps) == 0 {
|
||||
s.sliceCaps[&dummyCallExpr] = sliceMap
|
||||
return
|
||||
}
|
||||
|
||||
params := funcDecl.Type.Params.List
|
||||
if len(params) > len(argCaps) {
|
||||
return // Length of params and args doesn't match, so don't do anything with this.
|
||||
}
|
||||
|
||||
for it := range params {
|
||||
capacity := argCaps[it]
|
||||
if capacity == nil {
|
||||
continue
|
||||
}
|
||||
|
||||
if len(params[it].Names) == 0 {
|
||||
continue
|
||||
}
|
||||
|
||||
if paramName := params[it].Names[0]; paramName != nil {
|
||||
sliceMap[paramName.Name] = capacity
|
||||
}
|
||||
}
|
||||
|
||||
s.sliceCaps[&dummyCallExpr] = sliceMap
|
||||
}
|
||||
|
||||
// matchSliceMake matches calls to make() and stores the capacity of the new slice in the map to compare against future slice usage.
|
||||
func (s *sliceOutOfBounds) matchSliceMake(funcCall *ast.CallExpr, sliceName string, ctx *gosec.Context) (*issue.Issue, error) {
|
||||
_, funcName, err := gosec.GetCallInfo(funcCall, ctx)
|
||||
if err != nil || funcName != "make" {
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
var capacityArg int
|
||||
if len(funcCall.Args) < 2 {
|
||||
return nil, nil // No size passed
|
||||
} else if len(funcCall.Args) == 2 {
|
||||
capacityArg = 1
|
||||
} else if len(funcCall.Args) == 3 {
|
||||
capacityArg = 2
|
||||
} else {
|
||||
return nil, nil // Unexpected, args should always be 2 or 3
|
||||
}
|
||||
|
||||
// Check if the type of the slice is a map, since they should no be checked.
|
||||
if _, ok := funcCall.Args[0].(*ast.MapType); ok {
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
// Check and get the capacity of the slice passed to make. It must be a literal value, since we aren't evaluating the expression.
|
||||
sliceCapLit, ok := funcCall.Args[capacityArg].(*ast.BasicLit)
|
||||
if !ok {
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
capacity, err := gosec.GetInt(sliceCapLit)
|
||||
if err != nil {
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
caps := s.getSliceCapsForFunc(s.currentFuncName, sliceName, ctx)
|
||||
for callExpr := range caps {
|
||||
caps[callExpr] = &capacity
|
||||
}
|
||||
|
||||
s.updateSliceCaps(sliceName, caps)
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
// evaluateSliceExpr takes a slice expression and evaluates what the capacity of said slice is for each of the
|
||||
// calls to the current function. Returns map of the call expressions of each call to the current function to
|
||||
// the evaluated capacities.
|
||||
func (s *sliceOutOfBounds) evaluateSliceExpr(node *ast.SliceExpr, ctx *gosec.Context) map[*ast.CallExpr]*int64 {
|
||||
// Get ident to get name
|
||||
ident, ok := node.X.(*ast.Ident)
|
||||
if !ok {
|
||||
return nil
|
||||
}
|
||||
|
||||
// Get cap of old slice to calculate this new slice's cap
|
||||
caps := s.getSliceCapsForFunc(s.currentFuncName, ident.Name, ctx)
|
||||
for callExpr, oldCap := range caps {
|
||||
if oldCap == nil {
|
||||
continue
|
||||
}
|
||||
|
||||
// Get and check low value
|
||||
lowIdent, ok := node.Low.(*ast.BasicLit)
|
||||
if ok && lowIdent != nil {
|
||||
low, _ := gosec.GetInt(lowIdent)
|
||||
|
||||
newCap := *oldCap - low
|
||||
caps[callExpr] = &newCap
|
||||
} else if lowIdent == nil { // If no lower bound, capacity will be same
|
||||
continue
|
||||
}
|
||||
}
|
||||
|
||||
return caps
|
||||
}
|
||||
|
||||
// matchSliceAssignment matches slice assignments, calculates capacity of slice if possible to store it in map.
|
||||
func (s *sliceOutOfBounds) matchSliceAssignment(node *ast.SliceExpr, sliceName string, ctx *gosec.Context) (*issue.Issue, error) {
|
||||
// First do the normal match that verifies the slice expr is not out of bounds
|
||||
if i, err := s.matchSliceExpr(node, ctx); err != nil {
|
||||
return i, fmt.Errorf("There was an error while matching a slice expression to check slice bounds for %s: %w", sliceName, err)
|
||||
}
|
||||
|
||||
// Now that the assignment is (presumably) successfully, we can calculate the capacity and add this new slice to the map
|
||||
caps := s.evaluateSliceExpr(node, ctx)
|
||||
s.updateSliceCaps(sliceName, caps)
|
||||
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
// matchAssign matches checks if an assignment statement is making a slice, or if it is assigning a slice.
|
||||
func (s *sliceOutOfBounds) matchAssign(node *ast.AssignStmt, ctx *gosec.Context) (*issue.Issue, error) {
|
||||
// Check RHS for calls to make() so we can get the actual size of the slice
|
||||
for it, i := range node.Rhs {
|
||||
// Get the slice name so we can associate the cap with the slice in the map
|
||||
sliceIdent, ok := node.Lhs[it].(*ast.Ident)
|
||||
if !ok {
|
||||
return nil, nil
|
||||
}
|
||||
sliceName := sliceIdent.Name
|
||||
|
||||
switch expr := i.(type) {
|
||||
case *ast.CallExpr: // Check for and handle call to make()
|
||||
return s.matchSliceMake(expr, sliceName, ctx)
|
||||
case *ast.SliceExpr: // Handle assignments to a slice
|
||||
return s.matchSliceAssignment(expr, sliceName, ctx)
|
||||
}
|
||||
}
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
// matchSliceExpr validates that a given slice expression (eg, slice[10:30]) is not out of bounds.
|
||||
func (s *sliceOutOfBounds) matchSliceExpr(node *ast.SliceExpr, ctx *gosec.Context) (*issue.Issue, error) {
|
||||
// First get the slice name so we can check the size in our map
|
||||
ident, ok := node.X.(*ast.Ident)
|
||||
if !ok {
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
// Get slice cap from the map to compare it against high and low
|
||||
caps := s.getSliceCapsForFunc(s.currentFuncName, ident.Name, ctx)
|
||||
|
||||
for _, cap := range caps {
|
||||
if cap == nil {
|
||||
continue
|
||||
}
|
||||
|
||||
// Get and check high value
|
||||
highIdent, ok := node.High.(*ast.BasicLit)
|
||||
if ok && highIdent != nil {
|
||||
high, _ := gosec.GetInt(highIdent)
|
||||
if high > *cap {
|
||||
return ctx.NewIssue(node, s.ID(), s.What, s.Severity, s.Confidence), nil
|
||||
}
|
||||
}
|
||||
|
||||
// Get and check low value
|
||||
lowIdent, ok := node.Low.(*ast.BasicLit)
|
||||
if ok && lowIdent != nil {
|
||||
low, _ := gosec.GetInt(lowIdent)
|
||||
if low > *cap {
|
||||
return ctx.NewIssue(node, s.ID(), s.What, s.Severity, s.Confidence), nil
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
// matchIndexExpr validates that an index into a slice is not out of bounds.
|
||||
func (s *sliceOutOfBounds) matchIndexExpr(node *ast.IndexExpr, ctx *gosec.Context) (*issue.Issue, error) {
|
||||
// First get the slice name so we can check the size in our map
|
||||
ident, ok := node.X.(*ast.Ident)
|
||||
if !ok {
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
// Get slice cap from the map to compare it against high and low
|
||||
caps := s.getSliceCapsForFunc(s.currentFuncName, ident.Name, ctx)
|
||||
|
||||
for _, cap := range caps {
|
||||
if cap == nil {
|
||||
continue
|
||||
}
|
||||
// Get the index literal
|
||||
indexIdent, ok := node.Index.(*ast.BasicLit)
|
||||
if ok && indexIdent != nil {
|
||||
index, _ := gosec.GetInt(indexIdent)
|
||||
if index >= *cap {
|
||||
return ctx.NewIssue(node, s.ID(), s.What, s.Severity, s.Confidence), nil
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
// NewSliceBoundCheck attempts to find any slices being accessed out of bounds
|
||||
// by reslicing or by being indexed.
|
||||
func NewSliceBoundCheck(id string, _ gosec.Config) (gosec.Rule, []ast.Node) {
|
||||
sliceMap := make(map[*ast.CallExpr]map[string]*int64)
|
||||
|
||||
return &sliceOutOfBounds{
|
||||
sliceCaps: sliceMap,
|
||||
currentFuncName: "",
|
||||
funcCallArgs: make(map[string][]*int64),
|
||||
MetaData: issue.MetaData{
|
||||
ID: id,
|
||||
Severity: issue.Medium,
|
||||
Confidence: issue.Medium,
|
||||
What: "Potentially accessing slice out of bounds",
|
||||
},
|
||||
}, []ast.Node{(*ast.CallExpr)(nil), (*ast.FuncDecl)(nil), (*ast.AssignStmt)(nil), (*ast.SliceExpr)(nil), (*ast.IndexExpr)(nil)}
|
||||
}
|
|
@ -3861,55 +3861,55 @@ func main() {
|
|||
|
||||
}`}, 2, gosec.NewConfig()},
|
||||
{[]string{`
|
||||
package main
|
||||
package main
|
||||
|
||||
import "fmt"
|
||||
import "fmt"
|
||||
|
||||
func main() {
|
||||
func main() {
|
||||
|
||||
s := make([]byte, 0, 4)
|
||||
x := s[:2]
|
||||
y := x[:10]
|
||||
fmt.Println(y)
|
||||
}`}, 1, gosec.NewConfig()},
|
||||
s := make([]byte, 0, 4)
|
||||
x := s[:2]
|
||||
y := x[:10]
|
||||
fmt.Println(y)
|
||||
}`}, 1, gosec.NewConfig()},
|
||||
|
||||
{[]string{`
|
||||
package main
|
||||
package main
|
||||
|
||||
import "fmt"
|
||||
import "fmt"
|
||||
|
||||
func main() {
|
||||
func main() {
|
||||
|
||||
s := make([]int, 0, 4)
|
||||
doStuff(s)
|
||||
}
|
||||
s := make([]int, 0, 4)
|
||||
doStuff(s)
|
||||
}
|
||||
|
||||
func doStuff(x []int) {
|
||||
newSlice := x[:10]
|
||||
fmt.Println(newSlice)
|
||||
}`}, 1, gosec.NewConfig()},
|
||||
func doStuff(x []int) {
|
||||
newSlice := x[:10]
|
||||
fmt.Println(newSlice)
|
||||
}`}, 1, gosec.NewConfig()},
|
||||
{[]string{`
|
||||
package main
|
||||
package main
|
||||
|
||||
import "fmt"
|
||||
import "fmt"
|
||||
|
||||
func main() {
|
||||
func main() {
|
||||
|
||||
s := make([]int, 0, 30)
|
||||
doStuff(s)
|
||||
x := make([]int, 20)
|
||||
y := x[10:]
|
||||
doStuff(y)
|
||||
z := y[5:]
|
||||
doStuff(z)
|
||||
}
|
||||
s := make([]int, 0, 30)
|
||||
doStuff(s)
|
||||
x := make([]int, 20)
|
||||
y := x[10:]
|
||||
doStuff(y)
|
||||
z := y[5:]
|
||||
doStuff(z)
|
||||
}
|
||||
|
||||
func doStuff(x []int) {
|
||||
newSlice := x[:10]
|
||||
fmt.Println(newSlice)
|
||||
newSlice2 := x[:6]
|
||||
fmt.Println(newSlice2)
|
||||
}`}, 2, gosec.NewConfig()},
|
||||
func doStuff(x []int) {
|
||||
newSlice := x[:10]
|
||||
fmt.Println(newSlice)
|
||||
newSlice2 := x[:6]
|
||||
fmt.Println(newSlice2)
|
||||
}`}, 2, gosec.NewConfig()},
|
||||
{[]string{`
|
||||
package main
|
||||
|
||||
|
@ -3923,6 +3923,49 @@ func main() {
|
|||
},
|
||||
}
|
||||
fmt.Println(testMap)
|
||||
}`}, 0, gosec.NewConfig()},
|
||||
{[]string{`
|
||||
package main
|
||||
|
||||
import "fmt"
|
||||
|
||||
func main() {
|
||||
s := make([]byte, 0)
|
||||
if len(s) > 0 {
|
||||
fmt.Println(s[0])
|
||||
}
|
||||
}`}, 0, gosec.NewConfig()},
|
||||
{[]string{`
|
||||
package main
|
||||
|
||||
import "fmt"
|
||||
|
||||
func main() {
|
||||
s := make([]byte, 0)
|
||||
if len(s) > 0 {
|
||||
fmt.Println("fake test")
|
||||
}
|
||||
fmt.Println(s[0])
|
||||
}`}, 1, gosec.NewConfig()},
|
||||
{[]string{`
|
||||
package main
|
||||
|
||||
import "fmt"
|
||||
|
||||
func main() {
|
||||
s := make([]int, 16)
|
||||
for i := 0; i < 17; i++ {
|
||||
s = append(s, i)
|
||||
}
|
||||
if len(s) < 16 {
|
||||
fmt.Println(s[10:16])
|
||||
} else {
|
||||
fmt.Println(s[3:18])
|
||||
}
|
||||
fmt.Println(s[0])
|
||||
for i := range s {
|
||||
fmt.Println(s[i])
|
||||
}
|
||||
}`}, 0, gosec.NewConfig()},
|
||||
}
|
||||
)
|
||||
|
|
Loading…
Reference in a new issue