Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

G115 Struct Attribute Checks #1221

Merged
merged 2 commits into from
Sep 16, 2024
Merged

G115 Struct Attribute Checks #1221

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
480ace5
allow struct attributes checks
czechbol Sep 12, 2024
e64423b
fix explicit check results
czechbol Sep 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 58 additions & 28 deletions analyzers/conversion_overflow.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ type integer struct {
type rangeResult struct {
minValue int
maxValue uint
explixitPositiveVals []uint
explicitPositiveVals []uint
explicitNegativeVals []int
isRangeCheck bool
convertFound bool
Expand Down Expand Up @@ -271,7 +271,7 @@ func hasExplicitRangeCheck(instr *ssa.Convert, dstType string) bool {
if result.isRangeCheck {
minValue = max(minValue, &result.minValue)
maxValue = min(maxValue, &result.maxValue)
explicitPositiveVals = append(explicitPositiveVals, result.explixitPositiveVals...)
explicitPositiveVals = append(explicitPositiveVals, result.explicitPositiveVals...)
explicitNegativeVals = append(explicitNegativeVals, result.explicitNegativeVals...)
}
case *ssa.Call:
Expand Down Expand Up @@ -325,16 +325,17 @@ func getResultRange(ifInstr *ssa.If, instr *ssa.Convert, visitedIfs map[*ssa.If]
result.convertFound = true
result.minValue = max(result.minValue, thenBounds.minValue)
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 {
result.convertFound = true
result.minValue = max(result.minValue, elseBounds.minValue)
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
}

Expand All @@ -344,8 +345,15 @@ func updateResultFromBinOp(result *rangeResult, binOp *ssa.BinOp, instr *ssa.Con
operandsFlipped := false

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)
Expand All @@ -362,25 +370,12 @@ func updateResultFromBinOp(result *rangeResult, binOp *ssa.BinOp, instr *ssa.Con
if !successPathConvert {
break
}

// 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()))
}

updateExplicitValues(result, constVal)
case token.NEQ:
if successPathConvert {
break
}

// 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()))
}
updateExplicitValues(result, constVal)
}

if op == "neg" {
Expand All @@ -391,11 +386,19 @@ func updateResultFromBinOp(result *rangeResult, binOp *ssa.BinOp, instr *ssa.Con
result.maxValue = uint(min)
}
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) {
// If the success path has a conversion and the operands are not flipped, then the constant value is the maximum value.
if successPathConvert && !operandsFlipped {
Expand Down Expand Up @@ -439,6 +442,8 @@ func walkBranchForConvert(block *ssa.BasicBlock, instr *ssa.Convert, visitedIfs
if result.isRangeCheck {
bounds.minValue = toPtr(max(result.minValue, bounds.minValue))
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:
if v == instr.X {
Expand All @@ -463,9 +468,10 @@ func isRangeCheck(v ssa.Value, x ssa.Value) bool {
switch op := v.(type) {
case *ssa.BinOp:
switch op.Op {
case token.LSS, token.LEQ, token.GTR, token.GEQ,
token.EQL, token.NEQ:
return op.X == compareVal || op.Y == compareVal
case token.LSS, token.LEQ, token.GTR, token.GEQ, token.EQL, token.NEQ:
leftMatch := isSameOrRelated(op.X, compareVal)
rightMatch := isSameOrRelated(op.Y, compareVal)
return leftMatch || rightMatch
}
}
return false
Expand All @@ -475,12 +481,36 @@ func getRealValueFromOperation(v ssa.Value) (ssa.Value, string) {
switch v := v.(type) {
case *ssa.UnOp:
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, ""
}

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 {
if len(explicitPosVals) == 0 && len(explicitNegVals) == 0 {
return false
Expand Down
93 changes: 93 additions & 0 deletions testutils/g115_samples.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -716,4 +716,97 @@ func main() {
}
`,
}, 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()},
}
Loading