mirror of
https://github.com/securego/gosec.git
synced 2024-12-24 11:35:52 +00:00
Fix conversion overflow false positives when they are checked or pre-determined
Signed-off-by: czechbol <adamludes@gmail.com>
This commit is contained in:
parent
71e397b994
commit
bcec04e784
2 changed files with 182 additions and 0 deletions
|
@ -16,6 +16,7 @@ package analyzers
|
||||||
|
|
||||||
import (
|
import (
|
||||||
"fmt"
|
"fmt"
|
||||||
|
"go/token"
|
||||||
"regexp"
|
"regexp"
|
||||||
"strconv"
|
"strconv"
|
||||||
|
|
||||||
|
@ -49,6 +50,9 @@ func runConversionOverflow(pass *analysis.Pass) (interface{}, error) {
|
||||||
case *ssa.Convert:
|
case *ssa.Convert:
|
||||||
src := instr.X.Type().Underlying().String()
|
src := instr.X.Type().Underlying().String()
|
||||||
dst := instr.Type().Underlying().String()
|
dst := instr.Type().Underlying().String()
|
||||||
|
if isSafeConversion(instr) {
|
||||||
|
continue
|
||||||
|
}
|
||||||
if isIntOverflow(src, dst) {
|
if isIntOverflow(src, dst) {
|
||||||
issue := newIssue(pass.Analyzer.Name,
|
issue := newIssue(pass.Analyzer.Name,
|
||||||
fmt.Sprintf("integer overflow conversion %s -> %s", src, dst),
|
fmt.Sprintf("integer overflow conversion %s -> %s", src, dst),
|
||||||
|
@ -70,6 +74,93 @@ func runConversionOverflow(pass *analysis.Pass) (interface{}, error) {
|
||||||
return nil, nil
|
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 {
|
type integer struct {
|
||||||
signed bool
|
signed bool
|
||||||
size int
|
size int
|
||||||
|
|
|
@ -262,6 +262,97 @@ func main() {
|
||||||
var a uint8 = 13
|
var a uint8 = 13
|
||||||
b := int(a)
|
b := int(a)
|
||||||
fmt.Printf("%d\n", b)
|
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()},
|
}, 0, gosec.NewConfig()},
|
||||||
|
|
Loading…
Reference in a new issue