mirror of
https://github.com/securego/gosec.git
synced 2026-06-20 00:15:59 +02:00
fix(G602): avoid false positives for range-over-array indexing (#1531)
This change fixes a false positive in G602 when iterating over fixed-size arrays with range and indexing using the loop variable. Corrects loop bound normalization in SSA-based index analysis so offsets are not applied twice. Preserves true-positive detection for real out-of-bounds patterns (for example index + 1 at upper edge). Adds regression samples covering both: valid range-over-array indexing (no issue expected) invalid shifted indexing inside the same loop (issue expected) Signed-off-by: Cosmin Cojocar <cosmin@cojocar.ch>
This commit is contained in:
+43
-12
@@ -576,6 +576,37 @@ func (s *sliceBoundsState) extractIntValueIndexAddr(refinstr *ssa.IndexAddr, sli
|
||||
base, offset := decomposeIndex(refinstr.Index)
|
||||
var sliceIncr int
|
||||
|
||||
canNormalizeToBase := func(bin *ssa.BinOp) bool {
|
||||
if bin == nil || refinstr == nil {
|
||||
return false
|
||||
}
|
||||
binBlock := bin.Block()
|
||||
idxBlock := refinstr.Block()
|
||||
if binBlock == nil || idxBlock == nil {
|
||||
return false
|
||||
}
|
||||
if binBlock != idxBlock {
|
||||
return true
|
||||
}
|
||||
binPos := -1
|
||||
idxPos := -1
|
||||
for i, ins := range binBlock.Instrs {
|
||||
if ins == bin {
|
||||
binPos = i
|
||||
}
|
||||
if ins == refinstr {
|
||||
idxPos = i
|
||||
}
|
||||
if binPos >= 0 && idxPos >= 0 {
|
||||
break
|
||||
}
|
||||
}
|
||||
if binPos < 0 || idxPos < 0 {
|
||||
return false
|
||||
}
|
||||
return binPos < idxPos
|
||||
}
|
||||
|
||||
// Case 1: Base is a constant (e.g., s[0+3])
|
||||
if val, ok := GetConstantInt64(base); ok {
|
||||
finalIdx := int(val) + offset
|
||||
@@ -640,16 +671,15 @@ func (s *sliceBoundsState) extractIntValueIndexAddr(refinstr *ssa.IndexAddr, sli
|
||||
}
|
||||
maxV := limit + incr
|
||||
|
||||
// If the limit is on 'next' (i+1 < limit), it still bounds 'i'
|
||||
// In 'range n', i reaches n-1.
|
||||
// Here we use a heuristic: if we find an upper bound, check it.
|
||||
// If the limit is found on an incremented value (next or nBase != p),
|
||||
// normalize it back to the base loop variable before applying index offset.
|
||||
boundAdjust := 0
|
||||
if (v == next && base != next && canNormalizeToBase(bin)) || (v == nBase && nBase != p && base != nBase) {
|
||||
boundAdjust = -nOffset
|
||||
}
|
||||
|
||||
if bound == lowerUnbounded || bound == upperBounded {
|
||||
// Correct the max value of 'base' based on where the limit was found
|
||||
finalMaxV := maxV
|
||||
if v == nBase && nBase != p {
|
||||
// if i + nOffset < limit, then i < limit - nOffset
|
||||
finalMaxV = maxV - nOffset
|
||||
}
|
||||
finalMaxV := maxV + boundAdjust
|
||||
if !isSliceIndexInsideBounds(sliceCap+sliceIncr, finalMaxV+offset) {
|
||||
return finalMaxV + offset, nil
|
||||
}
|
||||
@@ -661,10 +691,11 @@ func (s *sliceBoundsState) extractIntValueIndexAddr(refinstr *ssa.IndexAddr, sli
|
||||
incr := -1 // extractLenBound only handles LSS for now
|
||||
maxV := limit + off + incr
|
||||
|
||||
finalMaxV := maxV
|
||||
if v == nBase && nBase != p {
|
||||
finalMaxV = maxV - nOffset
|
||||
boundAdjust := 0
|
||||
if (v == next && base != next && canNormalizeToBase(bin)) || (v == nBase && nBase != p && base != nBase) {
|
||||
boundAdjust = -nOffset
|
||||
}
|
||||
finalMaxV := maxV + boundAdjust
|
||||
if !isSliceIndexInsideBounds(sliceCap+sliceIncr, finalMaxV+offset) {
|
||||
return finalMaxV + offset, nil
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user