From c073629009897d89e03229bc81232c7375892086 Mon Sep 17 00:00:00 2001 From: Cosmin Cojocar Date: Sun, 28 Dec 2025 18:39:40 +0100 Subject: [PATCH] Improve slice bound check (#1442) Improve slice bound check to habdle bounded values and properly parse the address index only from references Signed-off-by: Cosmin Cojocar --- analyzers/slice_bounds.go | 71 +++++++++++++++++++++++++++------------ testutils/g602_samples.go | 35 +++++++++++++++++++ 2 files changed, 85 insertions(+), 21 deletions(-) diff --git a/analyzers/slice_bounds.go b/analyzers/slice_bounds.go index 2347af0..aab5514 100644 --- a/analyzers/slice_bounds.go +++ b/analyzers/slice_bounds.go @@ -36,6 +36,7 @@ const ( upperUnbounded unbounded upperBounded + bounded ) const maxDepth = 20 @@ -182,6 +183,22 @@ func runSliceBounds(pass *analysis.Pass) (interface{}, error) { delete(issues, instr) } } + case bounded: + switch tinstr := instr.(type) { + case *ssa.Slice: + lower, upper := extractSliceBounds(tinstr) + if isSliceInsideBounds(value, value, lower, upper) { + delete(issues, instr) + } + case *ssa.IndexAddr: + indexValue, err := extractIntValue(tinstr.Index.String()) + if err != nil { + break + } + if indexValue == value { + delete(issues, instr) + } + } } } else if nestedIfInstr, ok := instr.(*ssa.If); ok { for _, nestedBlock := range nestedIfInstr.Block().Succs { @@ -258,23 +275,24 @@ func trackSliceBounds(depth int, sliceCap int, slice ssa.Node, violations *[]ssa func extractIntValueIndexAddr(refinstr *ssa.IndexAddr, sliceCap int) (int, error) { var indexIncr, sliceIncr int + idxRefs := refinstr.Index.Referrers() + if idxRefs == nil { + return 0, errors.New("no found") + } + for _, instr := range *idxRefs { + switch instr := instr.(type) { + case *ssa.BinOp: + _, index, err := extractBinOpBound(instr) + if err != nil { + return 0, err + } + switch instr.Op { + case token.LSS: + indexIncr-- + } - for _, block := range refinstr.Block().Preds { - for _, instr := range block.Instrs { - switch instr := instr.(type) { - case *ssa.BinOp: - _, index, err := extractBinOpBound(instr) - if err != nil { - return 0, err - } - switch instr.Op { - case token.LSS: - indexIncr-- - } - - if !isSliceIndexInsideBounds(sliceCap+sliceIncr, index+indexIncr) { - return index, nil - } + if !isSliceIndexInsideBounds(sliceCap+sliceIncr, index+indexIncr) { + return index, nil } } } @@ -322,19 +340,28 @@ func checkAllSlicesBounds(depth int, sliceCap int, slice *ssa.Slice, violations 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 { + refs := []ssa.Instruction{} + if call.Referrers() != nil { + refs = append(refs, *call.Referrers()...) + } + depth := 0 + for len(refs) > 0 && depth < maxDepth { + newrefs := []ssa.Instruction{} + 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 } + newrefs = append(newrefs, ref) } } } + refs = newrefs + depth++ } + } } return nil, nil @@ -363,6 +390,8 @@ func invBound(bound bound) bound { return unbounded case unbounded: return upperBounded + case bounded: + return bounded default: return unbounded } @@ -386,7 +415,7 @@ func extractBinOpBound(binop *ssa.BinOp) (bound, int, error) { case token.GTR, token.GEQ: return lowerUnbounded, value, nil case token.EQL: - return upperBounded, value, nil + return bounded, value, nil case token.NEQ: return unbounded, value, nil } @@ -407,7 +436,7 @@ func extractBinOpBound(binop *ssa.BinOp) (bound, int, error) { case token.GTR, token.GEQ: return upperUnbounded, value, nil case token.EQL: - return upperBounded, value, nil + return bounded, value, nil case token.NEQ: return unbounded, value, nil } diff --git a/testutils/g602_samples.go b/testutils/g602_samples.go index 3f42c41..1390781 100644 --- a/testutils/g602_samples.go +++ b/testutils/g602_samples.go @@ -413,4 +413,39 @@ func main() { } `}, 1, gosec.NewConfig()}, + {[]string{` +package main +func main() { + args := []any{"1"} + switch len(args) - 1 { + case 1: + _ = args[1] + } +} +`}, 0, gosec.NewConfig()}, + {[]string{` +package main + +import "fmt" + +func main() { + value := "1234567890" + weight := []int{2, 3, 4, 5, 6, 7} + wLen := len(weight) + l := len(value) - 1 + addr := make([]any, 7) + sum := 0 + weight[2] = 3 + for i := l; i >= 0; i-- { + v := int(value[i] - '0') + if v < 0 || v > 9 { + fmt.Println("invalid number at column", i+1) + break + } + addr[2] = v + sum += v * weight[(l-i)%wLen] + } + fmt.Println(sum) +} +`}, 0, gosec.NewConfig()}, }