From 459e2d3e91bd0c6d4b989238a613b40cadd1ab1f Mon Sep 17 00:00:00 2001
From: Hiroki Suezawa <suezawa@gmail.com>
Date: Tue, 21 Jan 2020 18:13:12 +0900
Subject: [PATCH] Modify rule for integer overflow to have more acurate results
 (#434)

Signed-off-by: Hiroki Suezawa <suezawa@gmail.com>
---
 rules/integer_overflow.go | 21 +++++++++------------
 testutils/source.go       | 17 ++++++++++++++++-
 2 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/rules/integer_overflow.go b/rules/integer_overflow.go
index 702c02e..33a3139 100644
--- a/rules/integer_overflow.go
+++ b/rules/integer_overflow.go
@@ -30,32 +30,29 @@ func (i *integerOverflowCheck) ID() string {
 }
 
 func (i *integerOverflowCheck) Match(node ast.Node, ctx *gosec.Context) (*gosec.Issue, error) {
-	var atoiVarName map[string]ast.Node
+	var atoiVarObj map[*ast.Object]ast.Node
 
 	// To check multiple lines, ctx.PassedValues is used to store temporary data.
 	if _, ok := ctx.PassedValues[i.ID()]; !ok {
-		atoiVarName = make(map[string]ast.Node)
-		ctx.PassedValues[i.ID()] = atoiVarName
-	} else if pv, ok := ctx.PassedValues[i.ID()].(map[string]ast.Node); ok {
-		atoiVarName = pv
+		atoiVarObj = make(map[*ast.Object]ast.Node)
+		ctx.PassedValues[i.ID()] = atoiVarObj
+	} else if pv, ok := ctx.PassedValues[i.ID()].(map[*ast.Object]ast.Node); ok {
+		atoiVarObj = pv
 	} else {
-		return nil, fmt.Errorf("PassedValues[%s] of Context is not map[string]ast.Node, but %T", i.ID(), ctx.PassedValues[i.ID()])
+		return nil, fmt.Errorf("PassedValues[%s] of Context is not map[*ast.Object]ast.Node, but %T", i.ID(), ctx.PassedValues[i.ID()])
 	}
 
 	// strconv.Atoi is a common function.
 	// To reduce false positives, This rule detects code which is converted to int32/int16 only.
 	switch n := node.(type) {
-	case *ast.FuncDecl:
-		// Clear atoiVarName by function
-		ctx.PassedValues[i.ID()] = make(map[string]ast.Node)
 	case *ast.AssignStmt:
 		for _, expr := range n.Rhs {
 			if callExpr, ok := expr.(*ast.CallExpr); ok && i.calls.ContainsCallExpr(callExpr, ctx, false) != nil {
 				if idt, ok := n.Lhs[0].(*ast.Ident); ok && idt.Name != "_" {
 					// Example:
 					//  v, _ := strconv.Atoi("1111")
-					// Add "v" to atoiVarName map
-					atoiVarName[idt.Name] = n
+					// Add v's Obj to atoiVarObj map
+					atoiVarObj[idt.Obj] = n
 				}
 			}
 		}
@@ -63,7 +60,7 @@ func (i *integerOverflowCheck) Match(node ast.Node, ctx *gosec.Context) (*gosec.
 		if fun, ok := n.Fun.(*ast.Ident); ok {
 			if fun.Name == "int32" || fun.Name == "int16" {
 				if idt, ok := n.Args[0].(*ast.Ident); ok {
-					if n, ok := atoiVarName[idt.Name]; ok {
+					if n, ok := atoiVarObj[idt.Obj]; ok {
 						// Detect int32(v) and int16(v)
 						return gosec.NewIssue(ctx, n, i.ID(), i.What, i.Severity, i.Confidence), nil
 					}
diff --git a/testutils/source.go b/testutils/source.go
index 7c58da3..00d066f 100644
--- a/testutils/source.go
+++ b/testutils/source.go
@@ -525,7 +525,6 @@ func main() {
 
 	// SampleCodeG109 - Potential Integer OverFlow
 	SampleCodeG109 = []CodeSample{
-		// Bind to all networks explicitly
 		{[]string{`
 package main
 
@@ -592,6 +591,22 @@ func test() {
 	bigValue := 30
 	value := int32(bigValue)
 	fmt.Println(value)
+}`}, 0, gosec.NewConfig()}, {[]string{`
+package main
+
+import (
+	"fmt"
+	"strconv"
+)
+
+func main() {
+	value := 10
+	if value == 10 {
+		value, _ := strconv.Atoi("2147483648")
+		fmt.Println(value)
+	}
+	v := int32(value)
+	fmt.Println(v)
 }`}, 0, gosec.NewConfig()}}
 
 	// SampleCodeG110 - potential DoS vulnerability via decompression bomb