G115 Struct Attribute Checks (#1221)

* allow struct attributes checks

* fix explicit check results
This commit is contained in:
czechbol 2024-09-16 10:30:54 +02:00 committed by GitHub
parent 5f3194b581
commit 1f3bdd9349
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 151 additions and 28 deletions

View file

@ -40,7 +40,7 @@ type integer struct {
type rangeResult struct { type rangeResult struct {
minValue int minValue int
maxValue uint maxValue uint
explixitPositiveVals []uint explicitPositiveVals []uint
explicitNegativeVals []int explicitNegativeVals []int
isRangeCheck bool isRangeCheck bool
convertFound bool convertFound bool
@ -271,7 +271,7 @@ func hasExplicitRangeCheck(instr *ssa.Convert, dstType string) bool {
if result.isRangeCheck { if result.isRangeCheck {
minValue = max(minValue, &result.minValue) minValue = max(minValue, &result.minValue)
maxValue = min(maxValue, &result.maxValue) maxValue = min(maxValue, &result.maxValue)
explicitPositiveVals = append(explicitPositiveVals, result.explixitPositiveVals...) explicitPositiveVals = append(explicitPositiveVals, result.explicitPositiveVals...)
explicitNegativeVals = append(explicitNegativeVals, result.explicitNegativeVals...) explicitNegativeVals = append(explicitNegativeVals, result.explicitNegativeVals...)
} }
case *ssa.Call: case *ssa.Call:
@ -325,16 +325,17 @@ func getResultRange(ifInstr *ssa.If, instr *ssa.Convert, visitedIfs map[*ssa.If]
result.convertFound = true result.convertFound = true
result.minValue = max(result.minValue, thenBounds.minValue) result.minValue = max(result.minValue, thenBounds.minValue)
result.maxValue = min(result.maxValue, thenBounds.maxValue) result.maxValue = min(result.maxValue, thenBounds.maxValue)
result.explixitPositiveVals = append(result.explixitPositiveVals, thenBounds.explixitPositiveVals...)
result.explicitNegativeVals = append(result.explicitNegativeVals, thenBounds.explicitNegativeVals...)
} else if elseBounds.convertFound { } else if elseBounds.convertFound {
result.convertFound = true result.convertFound = true
result.minValue = max(result.minValue, elseBounds.minValue) result.minValue = max(result.minValue, elseBounds.minValue)
result.maxValue = min(result.maxValue, elseBounds.maxValue) result.maxValue = min(result.maxValue, elseBounds.maxValue)
result.explixitPositiveVals = append(result.explixitPositiveVals, elseBounds.explixitPositiveVals...)
result.explicitNegativeVals = append(result.explicitNegativeVals, elseBounds.explicitNegativeVals...)
} }
result.explicitPositiveVals = append(result.explicitPositiveVals, thenBounds.explixitPositiveVals...)
result.explicitNegativeVals = append(result.explicitNegativeVals, thenBounds.explicitNegativeVals...)
result.explicitPositiveVals = append(result.explicitPositiveVals, elseBounds.explixitPositiveVals...)
result.explicitNegativeVals = append(result.explicitNegativeVals, elseBounds.explicitNegativeVals...)
return result return result
} }
@ -344,8 +345,15 @@ func updateResultFromBinOp(result *rangeResult, binOp *ssa.BinOp, instr *ssa.Con
operandsFlipped := false operandsFlipped := false
compareVal, op := getRealValueFromOperation(instr.X) compareVal, op := getRealValueFromOperation(instr.X)
if x != compareVal {
y, operandsFlipped = x, true // Handle FieldAddr
if fieldAddr, ok := compareVal.(*ssa.FieldAddr); ok {
compareVal = fieldAddr
}
if !isSameOrRelated(x, compareVal) {
y = x
operandsFlipped = true
} }
constVal, ok := y.(*ssa.Const) constVal, ok := y.(*ssa.Const)
@ -362,25 +370,12 @@ func updateResultFromBinOp(result *rangeResult, binOp *ssa.BinOp, instr *ssa.Con
if !successPathConvert { if !successPathConvert {
break break
} }
updateExplicitValues(result, constVal)
// Determine if the constant value is positive or negative.
if strings.Contains(constVal.String(), "-") {
result.explicitNegativeVals = append(result.explicitNegativeVals, int(constVal.Int64()))
} else {
result.explixitPositiveVals = append(result.explixitPositiveVals, uint(constVal.Uint64()))
}
case token.NEQ: case token.NEQ:
if successPathConvert { if successPathConvert {
break break
} }
updateExplicitValues(result, constVal)
// Determine if the constant value is positive or negative.
if strings.Contains(constVal.String(), "-") {
result.explicitNegativeVals = append(result.explicitNegativeVals, int(constVal.Int64()))
} else {
result.explixitPositiveVals = append(result.explixitPositiveVals, uint(constVal.Uint64()))
}
} }
if op == "neg" { if op == "neg" {
@ -391,11 +386,19 @@ func updateResultFromBinOp(result *rangeResult, binOp *ssa.BinOp, instr *ssa.Con
result.maxValue = uint(min) result.maxValue = uint(min)
} }
if max <= math.MaxInt { if max <= math.MaxInt {
result.minValue = int(max) //nolint:gosec result.minValue = int(max)
} }
} }
} }
func updateExplicitValues(result *rangeResult, constVal *ssa.Const) {
if strings.Contains(constVal.String(), "-") {
result.explicitNegativeVals = append(result.explicitNegativeVals, int(constVal.Int64()))
} else {
result.explicitPositiveVals = append(result.explicitPositiveVals, uint(constVal.Uint64()))
}
}
func updateMinMaxForLessOrEqual(result *rangeResult, constVal *ssa.Const, op token.Token, operandsFlipped bool, successPathConvert bool) { func updateMinMaxForLessOrEqual(result *rangeResult, constVal *ssa.Const, op token.Token, operandsFlipped bool, successPathConvert bool) {
// If the success path has a conversion and the operands are not flipped, then the constant value is the maximum value. // If the success path has a conversion and the operands are not flipped, then the constant value is the maximum value.
if successPathConvert && !operandsFlipped { if successPathConvert && !operandsFlipped {
@ -439,6 +442,8 @@ func walkBranchForConvert(block *ssa.BasicBlock, instr *ssa.Convert, visitedIfs
if result.isRangeCheck { if result.isRangeCheck {
bounds.minValue = toPtr(max(result.minValue, bounds.minValue)) bounds.minValue = toPtr(max(result.minValue, bounds.minValue))
bounds.maxValue = toPtr(min(result.maxValue, bounds.maxValue)) bounds.maxValue = toPtr(min(result.maxValue, bounds.maxValue))
bounds.explixitPositiveVals = append(bounds.explixitPositiveVals, result.explicitPositiveVals...)
bounds.explicitNegativeVals = append(bounds.explicitNegativeVals, result.explicitNegativeVals...)
} }
case *ssa.Call: case *ssa.Call:
if v == instr.X { if v == instr.X {
@ -463,9 +468,10 @@ func isRangeCheck(v ssa.Value, x ssa.Value) bool {
switch op := v.(type) { switch op := v.(type) {
case *ssa.BinOp: case *ssa.BinOp:
switch op.Op { switch op.Op {
case token.LSS, token.LEQ, token.GTR, token.GEQ, case token.LSS, token.LEQ, token.GTR, token.GEQ, token.EQL, token.NEQ:
token.EQL, token.NEQ: leftMatch := isSameOrRelated(op.X, compareVal)
return op.X == compareVal || op.Y == compareVal rightMatch := isSameOrRelated(op.Y, compareVal)
return leftMatch || rightMatch
} }
} }
return false return false
@ -475,12 +481,36 @@ func getRealValueFromOperation(v ssa.Value) (ssa.Value, string) {
switch v := v.(type) { switch v := v.(type) {
case *ssa.UnOp: case *ssa.UnOp:
if v.Op == token.SUB { if v.Op == token.SUB {
return v.X, "neg" val, _ := getRealValueFromOperation(v.X)
return val, "neg"
} }
return getRealValueFromOperation(v.X)
case *ssa.FieldAddr:
return v, "field"
case *ssa.Alloc:
return v, "alloc"
} }
return v, "" return v, ""
} }
func isSameOrRelated(a, b ssa.Value) bool {
aVal, _ := getRealValueFromOperation(a)
bVal, _ := getRealValueFromOperation(b)
if aVal == bVal {
return true
}
// Check if both are FieldAddr operations referring to the same field of the same struct
if aField, aOk := aVal.(*ssa.FieldAddr); aOk {
if bField, bOk := bVal.(*ssa.FieldAddr); bOk {
return aField.X == bField.X && aField.Field == bField.Field
}
}
return false
}
func explicitValsInRange(explicitPosVals []uint, explicitNegVals []int, dstInt integer) bool { func explicitValsInRange(explicitPosVals []uint, explicitNegVals []int, dstInt integer) bool {
if len(explicitPosVals) == 0 && len(explicitNegVals) == 0 { if len(explicitPosVals) == 0 && len(explicitNegVals) == 0 {
return false return false

View file

@ -716,4 +716,97 @@ func main() {
} }
`, `,
}, 0, gosec.NewConfig()}, }, 0, gosec.NewConfig()},
{[]string{
`
package main
import (
"fmt"
"math"
)
type CustomStruct struct {
Value int
}
func main() {
results := CustomStruct{Value: 0}
if results.Value < math.MinInt32 || results.Value > math.MaxInt32 {
panic("value out of range for int32")
}
convertedValue := int32(results.Value)
fmt.Println(convertedValue)
}
`,
}, 0, gosec.NewConfig()},
{[]string{
`
package main
import (
"fmt"
"math"
)
type CustomStruct struct {
Value int
}
func main() {
results := CustomStruct{Value: 0}
if results.Value >= math.MinInt32 && results.Value <= math.MaxInt32 {
convertedValue := int32(results.Value)
fmt.Println(convertedValue)
}
panic("value out of range for int32")
}
`,
}, 0, gosec.NewConfig()},
{[]string{
`
package main
import (
"fmt"
"math"
)
type CustomStruct struct {
Value int
}
func main() {
results := CustomStruct{Value: 0}
if results.Value < math.MinInt32 || results.Value > math.MaxInt32 {
panic("value out of range for int32")
}
// checked value is decremented by 1 before conversion which is unsafe
convertedValue := int32(results.Value-1)
fmt.Println(convertedValue)
}
`,
}, 1, gosec.NewConfig()},
{[]string{
`
package main
import (
"fmt"
"math"
"math/rand"
)
func main() {
a := rand.Int63()
if a < math.MinInt32 || a > math.MaxInt32 {
panic("out of range")
}
// checked value is incremented by 1 before conversion which is unsafe
b := int32(a+1)
fmt.Printf("%d\n", b)
}
`,
}, 1, gosec.NewConfig()},
} }