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.

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

Fix conversion overflow false positives #1189

Merged
merged 1 commit into from
Aug 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
91 changes: 91 additions & 0 deletions analyzers/conversion_overflow.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package analyzers

import (
"fmt"
"go/token"
"regexp"
"strconv"

Expand Down Expand Up @@ -49,6 +50,9 @@ func runConversionOverflow(pass *analysis.Pass) (interface{}, error) {
case *ssa.Convert:
src := instr.X.Type().Underlying().String()
dst := instr.Type().Underlying().String()
if isSafeConversion(instr) {
continue
}
if isIntOverflow(src, dst) {
issue := newIssue(pass.Analyzer.Name,
fmt.Sprintf("integer overflow conversion %s -> %s", src, dst),
Expand All @@ -70,6 +74,93 @@ func runConversionOverflow(pass *analysis.Pass) (interface{}, error) {
return nil, nil
}

func isSafeConversion(instr *ssa.Convert) bool {
dstType := instr.Type().Underlying().String()

// Check for constant conversions
if constVal, ok := instr.X.(*ssa.Const); ok {
if isConstantInRange(constVal, dstType) {
return true
}
}

// Check for explicit range checks
if hasExplicitRangeCheck(instr) {
return true
}

// Check for string to integer conversions with specified bit size
if isStringToIntConversion(instr, dstType) {
return true
}

return false
}

func isConstantInRange(constVal *ssa.Const, dstType string) bool {
value, err := strconv.ParseInt(constVal.Value.String(), 10, 64)
if err != nil {
return false
}

dstInt, err := parseIntType(dstType)
if err != nil {
return false
}

if dstInt.signed {
return value >= -(1<<(dstInt.size-1)) && value <= (1<<(dstInt.size-1))-1
}
return value >= 0 && value <= (1<<dstInt.size)-1
}

func hasExplicitRangeCheck(instr *ssa.Convert) bool {
block := instr.Block()
for _, i := range block.Instrs {
if binOp, ok := i.(*ssa.BinOp); ok {
// Check if either operand of the BinOp is the result of the Convert instruction
if (binOp.X == instr || binOp.Y == instr) &&
(binOp.Op == token.LSS || binOp.Op == token.LEQ || binOp.Op == token.GTR || binOp.Op == token.GEQ) {
return true
}
}
}
return false
}

func isStringToIntConversion(instr *ssa.Convert, dstType string) bool {
// Traverse the SSA instructions to find the original variable
original := instr.X
for {
switch v := original.(type) {
case *ssa.Call:
if v.Call.StaticCallee() != nil && v.Call.StaticCallee().Name() == "ParseInt" {
if len(v.Call.Args) == 3 {
if bitSize, ok := v.Call.Args[2].(*ssa.Const); ok {
bitSizeValue, err := strconv.Atoi(bitSize.Value.String())
if err != nil {
return false
}
dstInt, err := parseIntType(dstType)
if err != nil {
return false
}
isSafe := bitSizeValue <= dstInt.size
return isSafe
}
}
}
return false
case *ssa.Phi:
original = v.Edges[0]
case *ssa.Extract:
original = v.Tuple
default:
return false
}
}
}

type integer struct {
signed bool
size int
Expand Down
91 changes: 91 additions & 0 deletions testutils/g115_samples.go
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,97 @@ func main() {
var a uint8 = 13
b := int(a)
fmt.Printf("%d\n", b)
}
`,
}, 0, gosec.NewConfig()},
{[]string{
`
package main

import (
"fmt"
)

func main() {
const a int64 = 13
b := int32(a)
fmt.Printf("%d\n", b)
}
`,
}, 0, gosec.NewConfig()},
{[]string{
`
package main

import (
"fmt"
"math"
)

func main() {
var a int64 = 13
if a < math.MinInt32 || a > math.MaxInt32 {
panic("out of range")
}
b := int32(a)
fmt.Printf("%d\n", b)
}
`,
}, 0, gosec.NewConfig()},
{[]string{
`
package main

import (
"fmt"
"math"
"math/rand"
)

func main() {
a := rand.Int63()
if a < math.MinInt64 || a > math.MaxInt32 {
panic("out of range")
}
b := int32(a)
fmt.Printf("%d\n", b)
}
`,
}, 1, gosec.NewConfig()},
{[]string{
`
package main

import (
"fmt"
"math"
)

func main() {
var a int32 = math.MaxInt32
if a < math.MinInt32 || a > math.MaxInt32 {
panic("out of range")
}
var b int64 = int64(a) * 2
c := int32(b)
fmt.Printf("%d\n", c)
}
`,
}, 1, gosec.NewConfig()},
{[]string{
`
package main

import (
"fmt"
"strconv"
)

func main() {
var a string = "13"
b, _ := strconv.ParseInt(a, 10, 32)
c := int32(b)
fmt.Printf("%d\n", c)
}
`,
}, 0, gosec.NewConfig()},
Expand Down
Loading