fix: G602 support for nested conditionals with bounds check (#1201)

* Recursive fix

* Add some more test cases

* Fix formatting

* Add depth check
This commit is contained in:
William Bergeron-Drouin 2024-09-04 05:07:42 -04:00 committed by GitHub
parent 11d69032b0
commit ea5b2766bb
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 133 additions and 32 deletions

View file

@ -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)
}
}

View file

@ -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{`
@ -210,10 +210,10 @@ 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{`
@ -221,6 +221,93 @@ 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 {
@ -249,5 +336,6 @@ func main() {
fmt.Println(s[i])
}
}
`}, 0, gosec.NewConfig()},
}