Skip to content

Commit

Permalink
fix: G602 support for nested conditionals with bounds check (#1201)
Browse files Browse the repository at this point in the history
* Recursive fix

* Add some more test cases

* Fix formatting

* Add depth check
  • Loading branch information
xWiiLLz authored Sep 4, 2024
1 parent 11d6903 commit ea5b276
Show file tree
Hide file tree
Showing 2 changed files with 133 additions and 32 deletions.
55 changes: 34 additions & 21 deletions analyzers/slice_bounds.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,32 +118,45 @@ func runSliceBounds(pass *analysis.Pass) (interface{}, error) {
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)
var processBlock func(block *ssa.BasicBlock, depth int)
processBlock = func(block *ssa.BasicBlock, depth int) {
if depth == maxDepth {
return
}
depth++
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)
}
}
}
} else if nestedIfInstr, ok := instr.(*ssa.If); ok {
for _, nestedBlock := range nestedIfInstr.Block().Succs {
processBlock(nestedBlock, depth)
}
}
}
}

processBlock(block, 0)
}
}

Expand Down
110 changes: 99 additions & 11 deletions testutils/g602_samples.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,13 +195,13 @@ package main
import "fmt"
func main() {
testMap := make(map[string]any, 0)
testMap["test1"] = map[string]interface{}{
"test2": map[string]interface{}{
"value": 0,
},
}
fmt.Println(testMap)
testMap := make(map[string]any, 0)
testMap["test1"] = map[string]interface{}{
"test2": map[string]interface{}{
"value": 0,
},
}
fmt.Println(testMap)
}
`}, 0, gosec.NewConfig()},
{[]string{`
Expand All @@ -210,17 +210,104 @@ package main
import "fmt"
func main() {
s := make([]byte, 0)
if len(s) > 0 {
fmt.Println(s[0])
}
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 {
switch s[0] {
case 0:
fmt.Println("zero")
return
default:
fmt.Println(s[0])
return
}
}
}
`}, 0, gosec.NewConfig()},
{[]string{`
package main
import "fmt"
func main() {
s := make([]byte, 0)
if len(s) > 0 {
switch s[0] {
case 0:
b := true
if b == true {
// Should work for many-levels of nesting when the condition is not on the target slice
fmt.Println(s[0])
}
return
default:
fmt.Println(s[0])
return
}
}
}
`}, 0, gosec.NewConfig()},
{[]string{`
package main
import "fmt"
func main() {
s := make([]byte, 0)
if len(s) > 0 {
if len(s) > 1 {
fmt.Println(s[1])
}
fmt.Println(s[0])
}
}
`}, 0, gosec.NewConfig()},
{[]string{`
package main
import "fmt"
func main() {
s := make([]byte, 2)
fmt.Println(s[1])
s = make([]byte, 0)
fmt.Println(s[1])
}
`}, 1, gosec.NewConfig()},
{[]string{`
package main
import "fmt"
func main() {
s := make([]byte, 0)
if len(s) > 0 {
if len(s) > 4 {
fmt.Println(s[3])
} else {
// Should error
fmt.Println(s[2])
}
fmt.Println(s[0])
}
}
`}, 1, gosec.NewConfig()},
{[]string{`
package main
import "fmt"
func main() {
s := make([]byte, 0)
if len(s) > 0 {
Expand Down Expand Up @@ -249,5 +336,6 @@ func main() {
fmt.Println(s[i])
}
}
`}, 0, gosec.NewConfig()},
}

0 comments on commit ea5b276

Please sign in to comment.