Skip to content

Commit 6ab531c

Browse files
committed
Fix conversion overflow false positives when they are checked or pre-determined
Signed-off-by: czechbol <[email protected]>
1 parent ab3f6c1 commit 6ab531c

File tree

2 files changed

+186
-1
lines changed

2 files changed

+186
-1
lines changed

analyzers/conversion_overflow.go

Lines changed: 95 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ package analyzers
1616

1717
import (
1818
"fmt"
19+
"go/token"
1920
"regexp"
2021
"strconv"
2122

@@ -49,6 +50,9 @@ func runConversionOverflow(pass *analysis.Pass) (interface{}, error) {
4950
case *ssa.Convert:
5051
src := instr.X.Type().Underlying().String()
5152
dst := instr.Type().Underlying().String()
53+
if isSafeConversion(instr) {
54+
continue
55+
}
5256
if isIntOverflow(src, dst) {
5357
issue := newIssue(pass.Analyzer.Name,
5458
fmt.Sprintf("integer overflow conversion %s -> %s", src, dst),
@@ -70,6 +74,93 @@ func runConversionOverflow(pass *analysis.Pass) (interface{}, error) {
7074
return nil, nil
7175
}
7276

77+
func isSafeConversion(instr *ssa.Convert) bool {
78+
dstType := instr.Type().Underlying().String()
79+
80+
// Check for constant conversions
81+
if constVal, ok := instr.X.(*ssa.Const); ok {
82+
if isConstantInRange(constVal, dstType) {
83+
return true
84+
}
85+
}
86+
87+
// Check for explicit range checks
88+
if hasExplicitRangeCheck(instr) {
89+
return true
90+
}
91+
92+
// Check for string to integer conversions with specified bit size
93+
if isStringToIntConversion(instr, dstType) {
94+
return true
95+
}
96+
97+
return false
98+
}
99+
100+
func isConstantInRange(constVal *ssa.Const, dstType string) bool {
101+
value, err := strconv.ParseInt(constVal.Value.String(), 10, 64)
102+
if err != nil {
103+
return false
104+
}
105+
106+
dstInt, err := parseIntType(dstType)
107+
if err != nil {
108+
return false
109+
}
110+
111+
if dstInt.signed {
112+
return value >= -(1<<(dstInt.size-1)) && value <= (1<<(dstInt.size-1))-1
113+
}
114+
return value >= 0 && value <= (1<<dstInt.size)-1
115+
}
116+
117+
func hasExplicitRangeCheck(instr *ssa.Convert) bool {
118+
block := instr.Block()
119+
for _, i := range block.Instrs {
120+
if binOp, ok := i.(*ssa.BinOp); ok {
121+
// Check if either operand of the BinOp is the result of the Convert instruction
122+
if (binOp.X == instr || binOp.Y == instr) &&
123+
(binOp.Op == token.LSS || binOp.Op == token.LEQ || binOp.Op == token.GTR || binOp.Op == token.GEQ) {
124+
return true
125+
}
126+
}
127+
}
128+
return false
129+
}
130+
131+
func isStringToIntConversion(instr *ssa.Convert, dstType string) bool {
132+
// Traverse the SSA instructions to find the original variable
133+
original := instr.X
134+
for {
135+
switch v := original.(type) {
136+
case *ssa.Call:
137+
if v.Call.StaticCallee() != nil && v.Call.StaticCallee().Name() == "ParseInt" {
138+
if len(v.Call.Args) == 3 {
139+
if bitSize, ok := v.Call.Args[2].(*ssa.Const); ok {
140+
bitSizeValue, err := strconv.Atoi(bitSize.Value.String())
141+
if err != nil {
142+
return false
143+
}
144+
dstInt, err := parseIntType(dstType)
145+
if err != nil {
146+
return false
147+
}
148+
isSafe := bitSizeValue <= dstInt.size
149+
return isSafe
150+
}
151+
}
152+
}
153+
return false
154+
case *ssa.Phi:
155+
original = v.Edges[0]
156+
case *ssa.Extract:
157+
original = v.Tuple
158+
default:
159+
return false
160+
}
161+
}
162+
}
163+
73164
type integer struct {
74165
signed bool
75166
size int
@@ -100,7 +191,10 @@ func parseIntType(intType string) (integer, error) {
100191
}
101192
}
102193

103-
return integer{signed: signed, size: intSize}, nil
194+
return integer{
195+
signed: signed,
196+
size: intSize,
197+
}, nil
104198
}
105199

106200
func isIntOverflow(src string, dst string) bool {

testutils/g115_samples.go

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,97 @@ func main() {
262262
var a uint8 = 13
263263
b := int(a)
264264
fmt.Printf("%d\n", b)
265+
}
266+
`,
267+
}, 0, gosec.NewConfig()},
268+
{[]string{
269+
`
270+
package main
271+
272+
import (
273+
"fmt"
274+
)
275+
276+
func main() {
277+
const a int64 = 13
278+
b := int32(a)
279+
fmt.Printf("%d\n", b)
280+
}
281+
`,
282+
}, 0, gosec.NewConfig()},
283+
{[]string{
284+
`
285+
package main
286+
287+
import (
288+
"fmt"
289+
"math"
290+
)
291+
292+
func main() {
293+
var a int64 = 13
294+
if a < math.MinInt32 || a > math.MaxInt32 {
295+
panic("out of range")
296+
}
297+
b := int32(a)
298+
fmt.Printf("%d\n", b)
299+
}
300+
`,
301+
}, 0, gosec.NewConfig()},
302+
{[]string{
303+
`
304+
package main
305+
306+
import (
307+
"fmt"
308+
"math"
309+
"math/rand"
310+
)
311+
312+
func main() {
313+
a := rand.Int63()
314+
if a < math.MinInt64 || a > math.MaxInt32 {
315+
panic("out of range")
316+
}
317+
b := int32(a)
318+
fmt.Printf("%d\n", b)
319+
}
320+
`,
321+
}, 1, gosec.NewConfig()},
322+
{[]string{
323+
`
324+
package main
325+
326+
import (
327+
"fmt"
328+
"math"
329+
)
330+
331+
func main() {
332+
var a int32 = math.MaxInt32
333+
if a < math.MinInt32 || a > math.MaxInt32 {
334+
panic("out of range")
335+
}
336+
var b int64 = int64(a) * 2
337+
c := int32(b)
338+
fmt.Printf("%d\n", c)
339+
}
340+
`,
341+
}, 1, gosec.NewConfig()},
342+
{[]string{
343+
`
344+
package main
345+
346+
import (
347+
"fmt"
348+
"strconv"
349+
)
350+
351+
func main() {
352+
var a string = "13"
353+
b, _ := strconv.ParseInt(a, 10, 32)
354+
c := int32(b)
355+
fmt.Printf("%d\n", c)
265356
}
266357
`,
267358
}, 0, gosec.NewConfig()},

0 commit comments

Comments
 (0)