Skip to content

Commit

Permalink
Fix conversion overflow false positives when they are checked or pre-…
Browse files Browse the repository at this point in the history
…determined

Signed-off-by: czechbol <[email protected]>
  • Loading branch information
czechbol authored and ccojocar committed Aug 26, 2024
1 parent 71e397b commit bcec04e
Show file tree
Hide file tree
Showing 2 changed files with 182 additions and 0 deletions.
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

0 comments on commit bcec04e

Please sign in to comment.