From ad0cd6d226d033294319c585bce129616f49f1c9 Mon Sep 17 00:00:00 2001 From: Kondratev Pavel Date: Tue, 30 Sep 2025 04:25:07 +0500 Subject: [PATCH 1/7] check nil slices, partially check bounds --- analyzers/slice_bounds.go | 62 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 60 insertions(+), 2 deletions(-) diff --git a/analyzers/slice_bounds.go b/analyzers/slice_bounds.go index 968102f268..c1c8c74518 100644 --- a/analyzers/slice_bounds.go +++ b/analyzers/slice_bounds.go @@ -104,6 +104,22 @@ func runSliceBounds(pass *analysis.Pass) (interface{}, error) { } } } + case *ssa.Store: // check store to nil slice or out of bounds + case *ssa.IndexAddr: + if constantValue, ok := instr.X.(*ssa.Const); ok && constantValue.Type().String()[:2] == "[]" { + if constantValue.Value == nil { + issue := newIssue( + pass.Analyzer.Name, + "slice index out of range", + pass.Fset, + instr.Pos(), + issue.Low, + issue.High) + issues[instr] = issue + + break + } + } } } } @@ -191,7 +207,11 @@ func trackSliceBounds(depth int, sliceCap int, slice ssa.Node, violations *[]ssa trackSliceBounds(depth, newCap, refinstr, violations, ifs) } case *ssa.IndexAddr: - indexValue, err := extractIntValue(refinstr.Index.String()) + indexValue, err := extractIntValue(refinstr.Index.String()) //lower bound + if err == nil && !isSliceIndexInsideBounds(0, sliceCap, indexValue) { + *violations = append(*violations, refinstr) + } + indexValue, err = extractIntValueIndexAddr(refinstr, sliceCap) //lower bound if err == nil && !isSliceIndexInsideBounds(0, sliceCap, indexValue) { *violations = append(*violations, refinstr) } @@ -217,6 +237,22 @@ func trackSliceBounds(depth int, sliceCap int, slice ssa.Node, violations *[]ssa } } +func extractIntValueIndexAddr(refinstr *ssa.IndexAddr, sliceCap int) (int, error) { + 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 && !isSliceIndexInsideBounds(0, sliceCap, index) { + return index, nil + } + } + } + } + + return 0, errors.New("no found") +} + func checkAllSlicesBounds(depth int, sliceCap int, slice *ssa.Slice, violations *[]ssa.Instruction, ifs map[ssa.If]*ssa.BinOp) { if depth == maxDepth { return @@ -370,6 +406,10 @@ func extractSliceBounds(slice *ssa.Slice) (int, int) { } func extractIntValue(value string) (int, error) { + if i, err := extractIntValuePhi(value); err == nil { + return i, nil + } + parts := strings.Split(value, ":") if len(parts) != 2 { return 0, fmt.Errorf("invalid value: %s", value) @@ -381,7 +421,7 @@ func extractIntValue(value string) (int, error) { } func extractSliceCapFromAlloc(instr string) (int, error) { - re := regexp.MustCompile(`new \[(\d+)\]*`) + re := regexp.MustCompile(`new \[(\d+)\].*`) var sliceCap int matches := re.FindAllStringSubmatch(instr, -1) if matches == nil { @@ -397,3 +437,21 @@ func extractSliceCapFromAlloc(instr string) (int, error) { return 0, errors.New("no slice cap found") } + +func extractIntValuePhi(value string) (int, error) { + re := regexp.MustCompile(`phi \[.+: (\d+):.+, .*\].*`) + var sliceCap int + matches := re.FindAllStringSubmatch(value, -1) + if matches == nil { + return sliceCap, fmt.Errorf("invalid value: %s", value) + } + + if len(matches) > 0 { + m := matches[0] + if len(m) > 1 { + return strconv.Atoi(m[1]) + } + } + + return 0, fmt.Errorf("invalid value: %s", value) +} From 01d314521874cbc46d37bc7e5814ab6e43d577ce Mon Sep 17 00:00:00 2001 From: Kondratev Pavel Date: Wed, 1 Oct 2025 02:09:32 +0500 Subject: [PATCH 2/7] add tests, cleanup, add fixed array --- analyzers/slice_bounds.go | 92 ++++++++++++++++++++++++++++++++------- testutils/g602_samples.go | 75 +++++++++++++++++++++++++++++++ 2 files changed, 152 insertions(+), 15 deletions(-) diff --git a/analyzers/slice_bounds.go b/analyzers/slice_bounds.go index c1c8c74518..7c5e4370e0 100644 --- a/analyzers/slice_bounds.go +++ b/analyzers/slice_bounds.go @@ -104,21 +104,44 @@ func runSliceBounds(pass *analysis.Pass) (interface{}, error) { } } } - case *ssa.Store: // check store to nil slice or out of bounds case *ssa.IndexAddr: - if constantValue, ok := instr.X.(*ssa.Const); ok && constantValue.Type().String()[:2] == "[]" { - if constantValue.Value == nil { - issue := newIssue( - pass.Analyzer.Name, - "slice index out of range", - pass.Fset, - instr.Pos(), - issue.Low, - issue.High) - issues[instr] = issue + switch indexInstr := instr.X.(type) { + case *ssa.Const: + if indexInstr.Type().String()[:2] == "[]" { + if indexInstr.Value == nil { + issue := newIssue( + pass.Analyzer.Name, + "slice index out of range", + pass.Fset, + instr.Pos(), + issue.Low, + issue.High) + issues[instr] = issue + + break + } + } + case *ssa.Alloc: + typeStr := indexInstr.Type().String() + fmt.Println(typeStr) + arrayLen, err := extractArrayAllocValue(typeStr) // preallocated array + if err != nil { + break + } + _, err = extractIntValueIndexAddr(instr, arrayLen) + if err != nil { break } + + issue := newIssue( + pass.Analyzer.Name, + "slice index out of range", + pass.Fset, + instr.Pos(), + issue.Low, + issue.High) + issues[instr] = issue } } } @@ -207,11 +230,11 @@ func trackSliceBounds(depth int, sliceCap int, slice ssa.Node, violations *[]ssa trackSliceBounds(depth, newCap, refinstr, violations, ifs) } case *ssa.IndexAddr: - indexValue, err := extractIntValue(refinstr.Index.String()) //lower bound + indexValue, err := extractIntValue(refinstr.Index.String()) if err == nil && !isSliceIndexInsideBounds(0, sliceCap, indexValue) { *violations = append(*violations, refinstr) } - indexValue, err = extractIntValueIndexAddr(refinstr, sliceCap) //lower bound + indexValue, err = extractIntValueIndexAddr(refinstr, sliceCap) if err == nil && !isSliceIndexInsideBounds(0, sliceCap, indexValue) { *violations = append(*violations, refinstr) } @@ -238,12 +261,25 @@ func trackSliceBounds(depth int, sliceCap int, slice ssa.Node, violations *[]ssa } func extractIntValueIndexAddr(refinstr *ssa.IndexAddr, sliceCap int) (int, error) { + var ( + indexIncr, sliceIncr int + ) + 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 && !isSliceIndexInsideBounds(0, sliceCap, index) { + if err != nil { + return 0, err + } + switch instr.Op { // 1<2 1<=2 1>2 1>=2 + case token.LSS: + indexIncr-- + } + + fmt.Println(sliceCap+sliceIncr, index+indexIncr) + if !isSliceIndexInsideBounds(0, sliceCap+sliceIncr, index+indexIncr) { return index, nil } } @@ -339,9 +375,14 @@ func invBound(bound bound) bound { } } +var errExtractBinOp = fmt.Errorf("unable to extract constant from binop") + func extractBinOpBound(binop *ssa.BinOp) (bound, int, error) { if binop.X != nil { if x, ok := binop.X.(*ssa.Const); ok { + if x == nil { + return lowerUnbounded, 0, errExtractBinOp + } value, err := strconv.Atoi(x.Value.String()) if err != nil { return lowerUnbounded, value, err @@ -360,6 +401,9 @@ func extractBinOpBound(binop *ssa.BinOp) (bound, int, error) { } if binop.Y != nil { if y, ok := binop.Y.(*ssa.Const); ok { + if y == nil { + return lowerUnbounded, 0, errExtractBinOp + } value, err := strconv.Atoi(y.Value.String()) if err != nil { return lowerUnbounded, value, err @@ -376,7 +420,7 @@ func extractBinOpBound(binop *ssa.BinOp) (bound, int, error) { } } } - return lowerUnbounded, 0, fmt.Errorf("unable to extract constant from binop") + return lowerUnbounded, 0, errExtractBinOp } func isSliceIndexInsideBounds(l, h int, index int) bool { @@ -455,3 +499,21 @@ func extractIntValuePhi(value string) (int, error) { return 0, fmt.Errorf("invalid value: %s", value) } + +func extractArrayAllocValue(value string) (int, error) { + re := regexp.MustCompile(`.*\[(\d+)\].*`) + var sliceCap int + matches := re.FindAllStringSubmatch(value, -1) + if matches == nil { + return sliceCap, fmt.Errorf("invalid value: %s", value) + } + + if len(matches) > 0 { + m := matches[0] + if len(m) > 1 { + return strconv.Atoi(m[1]) + } + } + + return 0, fmt.Errorf("invalid value: %s", value) +} diff --git a/testutils/g602_samples.go b/testutils/g602_samples.go index c0fee62949..3f42c41ca1 100644 --- a/testutils/g602_samples.go +++ b/testutils/g602_samples.go @@ -338,4 +338,79 @@ func main() { } `}, 0, gosec.NewConfig()}, + {[]string{` +package main + +func main() { + s := make([]int, 16) + for i := 10; i < 17; i++ { + s[i]=i + } +} + +`}, 1, gosec.NewConfig()}, + {[]string{` +package main + +func main() { + var s []int + for i := 10; i < 17; i++ { + s[i]=i + } +} + +`}, 1, gosec.NewConfig()}, + {[]string{` +package main + +func main() { + s := make([]int,5, 16) + for i := 1; i < 6; i++ { + s[i]=i + } +} + +`}, 1, gosec.NewConfig()}, + {[]string{` +package main + +func main() { + var s [20]int + for i := 10; i < 17; i++ { + s[i]=i + } +}`}, 0, gosec.NewConfig()}, + {[]string{` +package main + +func main() { + var s [20]int + for i := 1; i < len(s); i++ { + s[i]=i + } +} + +`}, 0, gosec.NewConfig()}, + {[]string{` +package main + +func main() { + var s [20]int + for i := 1; i <= len(s); i++ { + s[i]=i + } +} + +`}, 1, gosec.NewConfig()}, + {[]string{` +package main + +func main() { + var s [20]int + for i := 18; i <= 22; i++ { + s[i]=i + } +} + +`}, 1, gosec.NewConfig()}, } From a33ed62e95ca43e05451ba53a8437c5383a0e82a Mon Sep 17 00:00:00 2001 From: Kondratev Pavel Date: Wed, 1 Oct 2025 02:21:04 +0500 Subject: [PATCH 3/7] cleanup --- analyzers/slice_bounds.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/analyzers/slice_bounds.go b/analyzers/slice_bounds.go index 7c5e4370e0..4bd39a91e8 100644 --- a/analyzers/slice_bounds.go +++ b/analyzers/slice_bounds.go @@ -123,7 +123,6 @@ func runSliceBounds(pass *analysis.Pass) (interface{}, error) { } case *ssa.Alloc: typeStr := indexInstr.Type().String() - fmt.Println(typeStr) arrayLen, err := extractArrayAllocValue(typeStr) // preallocated array if err != nil { break @@ -273,12 +272,11 @@ func extractIntValueIndexAddr(refinstr *ssa.IndexAddr, sliceCap int) (int, error if err != nil { return 0, err } - switch instr.Op { // 1<2 1<=2 1>2 1>=2 + switch instr.Op { case token.LSS: indexIncr-- } - fmt.Println(sliceCap+sliceIncr, index+indexIncr) if !isSliceIndexInsideBounds(0, sliceCap+sliceIncr, index+indexIncr) { return index, nil } From 7ab989a00d99634e6021f52ce5c0266f42453f06 Mon Sep 17 00:00:00 2001 From: Kondratev Pavel Date: Wed, 1 Oct 2025 18:03:20 +0500 Subject: [PATCH 4/7] lint --- analyzers/slice_bounds.go | 32 +++++++++++++------------------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/analyzers/slice_bounds.go b/analyzers/slice_bounds.go index 4bd39a91e8..2465f34876 100644 --- a/analyzers/slice_bounds.go +++ b/analyzers/slice_bounds.go @@ -81,23 +81,21 @@ func runSliceBounds(pass *analysis.Pass) (interface{}, error) { for _, s := range violations { switch s := s.(type) { case *ssa.Slice: - issue := newIssue( + issues[s] = 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( + issues[s] = newIssue( pass.Analyzer.Name, "slice index out of range", pass.Fset, s.Pos(), issue.Low, issue.High) - issues[s] = issue } } } @@ -109,14 +107,13 @@ func runSliceBounds(pass *analysis.Pass) (interface{}, error) { case *ssa.Const: if indexInstr.Type().String()[:2] == "[]" { if indexInstr.Value == nil { - issue := newIssue( + issues[instr] = newIssue( pass.Analyzer.Name, "slice index out of range", pass.Fset, instr.Pos(), issue.Low, issue.High) - issues[instr] = issue break } @@ -133,14 +130,13 @@ func runSliceBounds(pass *analysis.Pass) (interface{}, error) { break } - issue := newIssue( + issues[instr] = newIssue( pass.Analyzer.Name, "slice index out of range", pass.Fset, instr.Pos(), issue.Low, issue.High) - issues[instr] = issue } } } @@ -181,7 +177,7 @@ func runSliceBounds(pass *analysis.Pass) (interface{}, error) { if err != nil { break } - if isSliceIndexInsideBounds(0, value, indexValue) { + if isSliceIndexInsideBounds(value, indexValue) { delete(issues, instr) } } @@ -199,8 +195,8 @@ func runSliceBounds(pass *analysis.Pass) (interface{}, error) { } foundIssues := []*issue.Issue{} - for _, issue := range issues { - foundIssues = append(foundIssues, issue) + for _, v := range issues { + foundIssues = append(foundIssues, v) } if len(foundIssues) > 0 { return foundIssues, nil @@ -230,11 +226,11 @@ func trackSliceBounds(depth int, sliceCap int, slice ssa.Node, violations *[]ssa } case *ssa.IndexAddr: indexValue, err := extractIntValue(refinstr.Index.String()) - if err == nil && !isSliceIndexInsideBounds(0, sliceCap, indexValue) { + if err == nil && !isSliceIndexInsideBounds(sliceCap, indexValue) { *violations = append(*violations, refinstr) } indexValue, err = extractIntValueIndexAddr(refinstr, sliceCap) - if err == nil && !isSliceIndexInsideBounds(0, sliceCap, indexValue) { + if err == nil && !isSliceIndexInsideBounds(sliceCap, indexValue) { *violations = append(*violations, refinstr) } case *ssa.Call: @@ -260,9 +256,7 @@ func trackSliceBounds(depth int, sliceCap int, slice ssa.Node, violations *[]ssa } func extractIntValueIndexAddr(refinstr *ssa.IndexAddr, sliceCap int) (int, error) { - var ( - indexIncr, sliceIncr int - ) + var indexIncr, sliceIncr int for _, block := range refinstr.Block().Preds { for _, instr := range block.Instrs { @@ -277,7 +271,7 @@ func extractIntValueIndexAddr(refinstr *ssa.IndexAddr, sliceCap int) (int, error indexIncr-- } - if !isSliceIndexInsideBounds(0, sliceCap+sliceIncr, index+indexIncr) { + if !isSliceIndexInsideBounds(sliceCap+sliceIncr, index+indexIncr) { return index, nil } } @@ -421,8 +415,8 @@ func extractBinOpBound(binop *ssa.BinOp) (bound, int, error) { return lowerUnbounded, 0, errExtractBinOp } -func isSliceIndexInsideBounds(l, h int, index int) bool { - return (l <= index && index < h) +func isSliceIndexInsideBounds(h int, index int) bool { + return (0 <= index && index < h) } func isSliceInsideBounds(l, h int, cl, ch int) bool { From 2b5a1ac4dcbec56bf9729fe38bf81180d35b5cd3 Mon Sep 17 00:00:00 2001 From: Kondratev Pavel Date: Wed, 1 Oct 2025 23:50:21 +0500 Subject: [PATCH 5/7] looks like go bug, add second check --- analyzers/slice_bounds.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/analyzers/slice_bounds.go b/analyzers/slice_bounds.go index 2465f34876..f315d844c6 100644 --- a/analyzers/slice_bounds.go +++ b/analyzers/slice_bounds.go @@ -372,7 +372,7 @@ var errExtractBinOp = fmt.Errorf("unable to extract constant from binop") func extractBinOpBound(binop *ssa.BinOp) (bound, int, error) { if binop.X != nil { if x, ok := binop.X.(*ssa.Const); ok { - if x == nil { + if x == nil || ok { return lowerUnbounded, 0, errExtractBinOp } value, err := strconv.Atoi(x.Value.String()) @@ -393,7 +393,7 @@ func extractBinOpBound(binop *ssa.BinOp) (bound, int, error) { } if binop.Y != nil { if y, ok := binop.Y.(*ssa.Const); ok { - if y == nil { + if y == nil || ok { return lowerUnbounded, 0, errExtractBinOp } value, err := strconv.Atoi(y.Value.String()) From 9db3796ae300bd14db88c3b0592d17fac3ecd9f4 Mon Sep 17 00:00:00 2001 From: Kondratev Pavel Date: Thu, 2 Oct 2025 13:23:42 +0500 Subject: [PATCH 6/7] ohh --- analyzers/slice_bounds.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/analyzers/slice_bounds.go b/analyzers/slice_bounds.go index f315d844c6..ea2bb8fd6b 100644 --- a/analyzers/slice_bounds.go +++ b/analyzers/slice_bounds.go @@ -372,7 +372,7 @@ var errExtractBinOp = fmt.Errorf("unable to extract constant from binop") func extractBinOpBound(binop *ssa.BinOp) (bound, int, error) { if binop.X != nil { if x, ok := binop.X.(*ssa.Const); ok { - if x == nil || ok { + if x.Value == nil { return lowerUnbounded, 0, errExtractBinOp } value, err := strconv.Atoi(x.Value.String()) @@ -393,7 +393,7 @@ func extractBinOpBound(binop *ssa.BinOp) (bound, int, error) { } if binop.Y != nil { if y, ok := binop.Y.(*ssa.Const); ok { - if y == nil || ok { + if y.Value == nil { return lowerUnbounded, 0, errExtractBinOp } value, err := strconv.Atoi(y.Value.String()) From 33cce1794cb2026892a44a736bf6b8e3f5ac0e38 Mon Sep 17 00:00:00 2001 From: Kondratev Pavel Date: Thu, 2 Oct 2025 20:42:17 +0500 Subject: [PATCH 7/7] check instruction position --- analyzers/slice_bounds.go | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/analyzers/slice_bounds.go b/analyzers/slice_bounds.go index ea2bb8fd6b..2347af07ea 100644 --- a/analyzers/slice_bounds.go +++ b/analyzers/slice_bounds.go @@ -119,24 +119,25 @@ func runSliceBounds(pass *analysis.Pass) (interface{}, error) { } } case *ssa.Alloc: - typeStr := indexInstr.Type().String() - arrayLen, err := extractArrayAllocValue(typeStr) // preallocated array - if err != nil { - break - } + if instr.Pos() > 0 { + typeStr := indexInstr.Type().String() + arrayLen, err := extractArrayAllocValue(typeStr) // preallocated array + if err != nil { + break + } - _, err = extractIntValueIndexAddr(instr, arrayLen) - if err != nil { - break + _, err = extractIntValueIndexAddr(instr, arrayLen) + if err != nil { + break + } + issues[instr] = newIssue( + pass.Analyzer.Name, + "slice index out of range", + pass.Fset, + instr.Pos(), + issue.Low, + issue.High) } - - issues[instr] = newIssue( - pass.Analyzer.Name, - "slice index out of range", - pass.Fset, - instr.Pos(), - issue.Low, - issue.High) } } }