rules/sdk: more accurately determine overflow for *int*(len(...)) by type & 32/64-bit architectures

rules/sdk/int_end_conversion_test.go

@@ -0,0 +1,65 @@
+package sdk
+
+import "testing"
+
+func TestCanOverflowChecks32Bits(t *testing.T) {
+	if !is32Bit {
+		t.Skip("Not running on a 64-bit machine!")
+	}
+
+	cases := []struct {
+		endKind      string
+		wantOverflow bool
+	}{
+		{"int8", true},
+		{"int16", true},
+		{"int32", false},
+		{"int64", false},
+		{"int", false},
+		{"uint8", true},
+		{"uint16", true},
+		{"uint32", false},
+		{"uint64", false},
+		{"uint", false},
+	}
+
+	for _, tt := range cases {
+		tt := tt
+		t.Run(tt.endKind, func(t *testing.T) {
+			if got := canLenOverflow32(tt.endKind); got != tt.wantOverflow {
+				t.Fatalf("Mismatch\n\tGot: %t\n\tWant:%t", got, tt.wantOverflow)
+			}
+		})
+	}
+}
+
+func TestCanOverflowChecks64Bits(t *testing.T) {
+	if is32Bit {
+		t.Skip("Not running on a 32-bit machine!")
+	}
+
+	cases := []struct {
+		endKind      string
+		wantOverflow bool
+	}{
+		{"int8", true},
+		{"int16", true},
+		{"int32", true},
+		{"int", false},
+		{"int64", false},
+		{"uint8", true},
+		{"uint16", true},
+		{"uint32", true},
+		{"uint64", false},
+		{"uint", false},
+	}
+
+	for _, tt := range cases {
+		tt := tt
+		t.Run(tt.endKind, func(t *testing.T) {
+			if got := canLenOverflow64(tt.endKind); got != tt.wantOverflow {
+				t.Fatalf("Mismatch\n\tGot: %t\n\tWant:%t", got, tt.wantOverflow)
+			}
+		})
+	}
+}

rules/sdk/integer.go

@@ -72,9 +72,23 @@ func (i *integerOverflowCheck) Match(node ast.Node, ctx *gosec.Context) (*gosec.
 		switch arg := arg.(type) {
 		case *ast.CallExpr:
 			// len() returns an int that is always >= 0, so it will fit in a uint, uint64, or int64.
-			if argFun, ok := arg.Fun.(*ast.Ident); ok && argFun.Name == "len" && (fun.Name == "uint" || fun.Name == "uint64" || fun.Name == "int64") {
-				return nil, nil
+			argFun, ok := arg.Fun.(*ast.Ident)
+			if !ok || argFun.Name != "len" {
+				break
+			}
+
+			// Please see the rules for determining if *int*(len(...)) can overflow
+			// as per: https://github.com/cosmos/gosec/issues/54
+			lenCanOverflow := canLenOverflow64
+			if is32Bit {
+				lenCanOverflow = canLenOverflow32
 			}
+
+			if lenCanOverflow(fun.Name) {
+				return gosec.NewIssue(ctx, n, i.ID(), i.What, i.Severity, i.Confidence), nil
+			}
+			return nil, nil
+
 		case *ast.SelectorExpr:
 			// If the argument is being cast to its underlying type, there's no risk.
 			if ctx.Info.TypeOf(arg).Underlying() == ctx.Info.TypeOf(fun) {
@@ -104,3 +118,76 @@ func NewIntegerCast(id string, conf gosec.Config) (gosec.Rule, []ast.Node) {
 		},
 	}, []ast.Node{(*ast.FuncDecl)(nil), (*ast.AssignStmt)(nil), (*ast.CallExpr)(nil)}
 }
+
+// Please see the rules at https://github.com/cosmos/gosec/issues/54
+func canLenOverflow64(destKind string) bool {
+	switch destKind {
+	case "int8", "uint8", "int16", "uint16":
+		return true
+
+	case "uint64":
+		// uint64([0, maxInt64])
+		return false
+
+	case "uint32":
+		// uint32([0, maxInt64])
+		return true
+
+	case "uint":
+		// uint => uint64 => uint64([0, maxInt64])
+		return false
+
+	case "int64":
+		// int64([0, maxInt64])
+		return false
+
+	case "int32":
+		// int32([0, maxInt64])
+		return true
+
+	case "int":
+		// int64([0, maxInt64])
+		return false
+
+	default:
+		return true
+	}
+}
+
+const s = 1
+const is32Bit = (^uint(s-1))>>32 == 0 // #nosec
+
+// Please see the rules at https://github.com/cosmos/gosec/issues/54
+func canLenOverflow32(destKind string) bool {
+	switch destKind {
+	case "int8", "uint8", "int16", "uint16":
+		return true
+
+	case "uint64":
+		// uint64([0, maxInt32])
+		return false
+
+	case "uint32":
+		// uint32([0, maxInt32])
+		return false
+
+	case "uint":
+		// uint => uint32 => uint32([0, maxInt32])
+		return false
+
+	case "int64":
+		// int64([0, maxInt32])
+		return false
+
+	case "int32":
+		// int32([0, maxInt32])
+		return false
+
+	case "int":
+		// int => int32 => int32([0, maxInt32])
+		return false
+
+	default:
+		return true
+	}
+}