Add static analysis for unsafe uint casting (#10318)

* Add static analysis for unsafe uint casting

* Fix violations of uintcast

* go mod tidy

* Add exclusion to nogo for darwin build

* Add test for math.Int

* Move some things to const so they are assured not to exceed int64

* Self review

* lint

* fix tests

* fix test

* Add init check for non 64 bit OS

* Move new deps from WORKSPACE to deps.bzl

* fix bazel build for go analysis runs

* Update BUILD.bazel

Remove TODO

* add math.AddInt method

* Add new test casts

* Add case where builtin functions and declared functions are covered

* Fix new findings

* cleanup

Co-authored-by: prylabs-bulldozer[bot] <58059840+prylabs-bulldozer[bot]@users.noreply.github.com>
Co-authored-by: Nishant Das <nishdas93@gmail.com>
This commit is contained in:
Preston Van Loon
2022-03-11 03:34:30 -06:00
committed by GitHub
parent 693cc79cc9
commit c1197d7881
99 changed files with 1081 additions and 220 deletions

View File

@@ -1,4 +1,4 @@
load("@prysm//tools/go:def.bzl", "go_library")
load("@prysm//tools/go:def.bzl", "go_library", "go_test")
go_library(
name = "go_default_library",
@@ -12,4 +12,15 @@ go_library(
],
)
# gazelle:exclude analyzer_test.go
go_test(
name = "go_default_test",
srcs = ["analyzer_test.go"],
data = glob(["testdata/**"]) + [
"@go_sdk//:files",
],
embed = [":go_default_library"],
deps = [
"//build/bazel:go_default_library",
"@org_golang_x_tools//go/analysis/analysistest:go_default_library",
],
)

View File

@@ -3,9 +3,18 @@ package comparesame
import (
"testing"
"github.com/prysmaticlabs/prysm/build/bazel"
"golang.org/x/tools/go/analysis/analysistest"
)
func TestAnalyzer(t *testing.T) {
analysistest.Run(t, analysistest.TestData(), Analyzer)
func init() {
if bazel.BuiltWithBazel() {
bazel.SetGoEnv()
}
}
func TestAnalyzer(t *testing.T) {
testdata := bazel.TestDataPath(t)
analysistest.TestData = func() string { return testdata }
analysistest.Run(t, testdata, Analyzer)
}

View File

@@ -1,8 +0,0 @@
load("@prysm//tools/go:def.bzl", "go_library")
go_library(
name = "go_default_library",
srcs = ["compare_len.go"],
importpath = "github.com/prysmaticlabs/prysm/tools/analyzers/comparesame/testdata",
visibility = ["//visibility:public"],
)

View File

@@ -1,4 +1,4 @@
load("@prysm//tools/go:def.bzl", "go_library")
load("@prysm//tools/go:def.bzl", "go_library", "go_test")
go_library(
name = "go_default_library",
@@ -12,4 +12,15 @@ go_library(
],
)
# gazelle:exclude analyzer_test.go
go_test(
name = "go_default_test",
srcs = ["analyzer_test.go"],
data = glob(["testdata/**"]) + [
"@go_sdk//:files",
],
embed = [":go_default_library"],
deps = [
"//build/bazel:go_default_library",
"@org_golang_x_tools//go/analysis/analysistest:go_default_library",
],
)

View File

@@ -3,9 +3,18 @@ package cryptorand
import (
"testing"
"github.com/prysmaticlabs/prysm/build/bazel"
"golang.org/x/tools/go/analysis/analysistest"
)
func TestAnalyzer(t *testing.T) {
analysistest.Run(t, analysistest.TestData(), Analyzer)
func init() {
if bazel.BuiltWithBazel() {
bazel.SetGoEnv()
}
}
func TestAnalyzer(t *testing.T) {
testdata := bazel.TestDataPath(t)
analysistest.TestData = func() string { return testdata }
analysistest.Run(t, testdata, Analyzer)
}

View File

@@ -1,11 +0,0 @@
load("@prysm//tools/go:def.bzl", "go_library")
go_library(
name = "go_default_library",
srcs = [
"custom_import.go",
"rand_new.go",
],
importpath = "github.com/prysmaticlabs/prysm/tools/analyzers/cryptorand/testdata",
visibility = ["//visibility:public"],
)

View File

@@ -1,4 +1,4 @@
load("@prysm//tools/go:def.bzl", "go_library")
load("@prysm//tools/go:def.bzl", "go_library", "go_test")
go_library(
name = "go_default_library",
@@ -15,4 +15,15 @@ go_library(
],
)
# gazelle:exclude analyzer_test.go
go_test(
name = "go_default_test",
srcs = ["analyzer_test.go"],
data = glob(["testdata/**"]) + [
"@go_sdk//:files",
],
embed = [":go_default_library"],
deps = [
"//build/bazel:go_default_library",
"@org_golang_x_tools//go/analysis/analysistest:go_default_library",
],
)

View File

@@ -3,9 +3,18 @@ package ineffassign
import (
"testing"
"github.com/prysmaticlabs/prysm/build/bazel"
"golang.org/x/tools/go/analysis/analysistest"
)
func TestAnalyzer(t *testing.T) {
analysistest.Run(t, analysistest.TestData(), Analyzer)
func init() {
if bazel.BuiltWithBazel() {
bazel.SetGoEnv()
}
}
func TestAnalyzer(t *testing.T) {
testdata := bazel.TestDataPath(t)
analysistest.TestData = func() string { return testdata }
analysistest.Run(t, testdata, Analyzer)
}

View File

@@ -1,8 +0,0 @@
load("@prysm//tools/go:def.bzl", "go_library")
go_library(
name = "go_default_library",
srcs = ["ctx_assignment.go"],
importpath = "github.com/prysmaticlabs/prysm/tools/analyzers/ineffassign/testdata",
visibility = ["//visibility:public"],
)

View File

@@ -176,7 +176,7 @@ func (s *gcSizes) Sizeof(T types.Type) int64 {
k := t.Kind()
if int(k) < len(basicSizes) {
if s := basicSizes[k]; s > 0 {
return int64(s)
return int64(s) // lint:ignore uintcast -- a byte will never exceed int64
}
}
if k == types.String {

View File

@@ -1,4 +1,4 @@
load("@prysm//tools/go:def.bzl", "go_library")
load("@prysm//tools/go:def.bzl", "go_library", "go_test")
go_library(
name = "go_default_library",
@@ -12,4 +12,15 @@ go_library(
],
)
# gazelle:exclude analyzer_test.go
go_test(
name = "go_default_test",
srcs = ["analyzer_test.go"],
data = glob(["testdata/**"]) + [
"@go_sdk//:files",
],
embed = [":go_default_library"],
deps = [
"//build/bazel:go_default_library",
"@org_golang_x_tools//go/analysis/analysistest:go_default_library",
],
)

View File

@@ -3,9 +3,18 @@ package nop
import (
"testing"
"github.com/prysmaticlabs/prysm/build/bazel"
"golang.org/x/tools/go/analysis/analysistest"
)
func TestAnalyzer(t *testing.T) {
analysistest.Run(t, analysistest.TestData(), Analyzer)
func init() {
if bazel.BuiltWithBazel() {
bazel.SetGoEnv()
}
}
func TestAnalyzer(t *testing.T) {
testdata := bazel.TestDataPath(t)
analysistest.TestData = func() string { return testdata }
analysistest.Run(t, testdata, Analyzer)
}

View File

@@ -1,8 +0,0 @@
load("@prysm//tools/go:def.bzl", "go_library")
go_library(
name = "go_default_library",
srcs = ["no_op.go"],
importpath = "github.com/prysmaticlabs/prysm/tools/analyzers/nop/testdata",
visibility = ["//visibility:public"],
)

View File

@@ -1,4 +1,4 @@
load("@prysm//tools/go:def.bzl", "go_library")
load("@prysm//tools/go:def.bzl", "go_library", "go_test")
go_library(
name = "go_default_library",
@@ -12,4 +12,15 @@ go_library(
],
)
# gazelle:exclude analyzer_test.go
go_test(
name = "go_default_test",
srcs = ["analyzer_test.go"],
data = glob(["testdata/**"]) + [
"@go_sdk//:files",
],
embed = [":go_default_library"],
deps = [
"//build/bazel:go_default_library",
"@org_golang_x_tools//go/analysis/analysistest:go_default_library",
],
)

View File

@@ -3,9 +3,18 @@ package properpermissions
import (
"testing"
"github.com/prysmaticlabs/prysm/build/bazel"
"golang.org/x/tools/go/analysis/analysistest"
)
func TestAnalyzer(t *testing.T) {
analysistest.Run(t, analysistest.TestData(), Analyzer)
func init() {
if bazel.BuiltWithBazel() {
bazel.SetGoEnv()
}
}
func TestAnalyzer(t *testing.T) {
testdata := bazel.TestDataPath(t)
analysistest.TestData = func() string { return testdata }
analysistest.Run(t, testdata, Analyzer)
}

View File

@@ -1,11 +0,0 @@
load("@prysm//tools/go:def.bzl", "go_library")
go_library(
name = "go_default_library",
srcs = [
"custom_imports.go",
"regular_imports.go",
],
importpath = "github.com/prysmaticlabs/prysm/tools/analyzers/properpermissions/testdata",
visibility = ["//visibility:public"],
)

View File

@@ -1,4 +1,4 @@
load("@prysm//tools/go:def.bzl", "go_library")
load("@prysm//tools/go:def.bzl", "go_library", "go_test")
go_library(
name = "go_default_library",
@@ -14,4 +14,15 @@ go_library(
],
)
# gazelle:exclude analyzer_test.go
go_test(
name = "go_default_test",
srcs = ["analyzer_test.go"],
data = glob(["testdata/**"]) + [
"@go_sdk//:files",
],
embed = [":go_default_library"],
deps = [
"//build/bazel:go_default_library",
"@org_golang_x_tools//go/analysis/analysistest:go_default_library",
],
)

View File

@@ -3,9 +3,18 @@ package recursivelock
import (
"testing"
"github.com/prysmaticlabs/prysm/build/bazel"
"golang.org/x/tools/go/analysis/analysistest"
)
func TestAnalyzer(t *testing.T) {
analysistest.Run(t, analysistest.TestData(), Analyzer)
func init() {
if bazel.BuiltWithBazel() {
bazel.SetGoEnv()
}
}
func TestAnalyzer(t *testing.T) {
testdata := bazel.TestDataPath(t)
analysistest.TestData = func() string { return testdata }
analysistest.Run(t, testdata, Analyzer)
}

View File

@@ -1,15 +0,0 @@
load("@prysm//tools/go:def.bzl", "go_library")
go_library(
name = "go_default_library",
srcs = [
"badlockswithmethods.go",
"badlockswithstructs.go",
"complexlocks.go",
"globallocks.go",
"nonrlocks.go",
"types.go",
],
importpath = "github.com/prysmaticlabs/prysm/tools/analyzers/recursivelock/testdata",
visibility = ["//visibility:public"],
)

View File

@@ -1,4 +1,4 @@
load("@prysm//tools/go:def.bzl", "go_library")
load("@prysm//tools/go:def.bzl", "go_library", "go_test")
go_library(
name = "go_default_library",
@@ -12,4 +12,15 @@ go_library(
],
)
# gazelle:exclude analyzer_test.go
go_test(
name = "go_default_test",
srcs = ["analyzer_test.go"],
data = glob(["testdata/**"]) + [
"@go_sdk//:files",
],
embed = [":go_default_library"],
deps = [
"//build/bazel:go_default_library",
"@org_golang_x_tools//go/analysis/analysistest:go_default_library",
],
)

View File

@@ -3,9 +3,18 @@ package shadowpredecl
import (
"testing"
"github.com/prysmaticlabs/prysm/build/bazel"
"golang.org/x/tools/go/analysis/analysistest"
)
func TestAnalyzer(t *testing.T) {
analysistest.Run(t, analysistest.TestData(), Analyzer)
func init() {
if bazel.BuiltWithBazel() {
bazel.SetGoEnv()
}
}
func TestAnalyzer(t *testing.T) {
testdata := bazel.TestDataPath(t)
analysistest.TestData = func() string { return testdata }
analysistest.Run(t, testdata, Analyzer)
}

View File

@@ -1,8 +0,0 @@
load("@prysm//tools/go:def.bzl", "go_library")
go_library(
name = "go_default_library",
srcs = ["shadow.go"],
importpath = "github.com/prysmaticlabs/prysm/tools/analyzers/shadowpredecl/testdata",
visibility = ["//visibility:public"],
)

View File

@@ -1,4 +1,4 @@
load("@prysm//tools/go:def.bzl", "go_library")
load("@prysm//tools/go:def.bzl", "go_library", "go_test")
go_library(
name = "go_default_library",
@@ -12,4 +12,15 @@ go_library(
],
)
# gazelle:exclude analyzer_test.go
go_test(
name = "go_default_test",
srcs = ["analyzer_test.go"],
data = glob(["testdata/**"]) + [
"@go_sdk//:files",
],
embed = [":go_default_library"],
deps = [
"//build/bazel:go_default_library",
"@org_golang_x_tools//go/analysis/analysistest:go_default_library",
],
)

View File

@@ -3,9 +3,18 @@ package slicedirect
import (
"testing"
"github.com/prysmaticlabs/prysm/build/bazel"
"golang.org/x/tools/go/analysis/analysistest"
)
func TestAnalyzer(t *testing.T) {
analysistest.Run(t, analysistest.TestData(), Analyzer)
func init() {
if bazel.BuiltWithBazel() {
bazel.SetGoEnv()
}
}
func TestAnalyzer(t *testing.T) {
testdata := bazel.TestDataPath(t)
analysistest.TestData = func() string { return testdata }
analysistest.Run(t, testdata, Analyzer)
}

View File

@@ -1,8 +0,0 @@
load("@prysm//tools/go:def.bzl", "go_library")
go_library(
name = "go_default_library",
srcs = ["slice.go"],
importpath = "github.com/prysmaticlabs/prysm/tools/analyzers/slicedirect/testdata",
visibility = ["//visibility:public"],
)

View File

@@ -0,0 +1,27 @@
load("@prysm//tools/go:def.bzl", "go_library", "go_test")
go_library(
name = "go_default_library",
srcs = ["analyzer.go"],
importpath = "github.com/prysmaticlabs/prysm/tools/analyzers/uintcast",
visibility = ["//visibility:public"],
deps = [
"@com_github_gostaticanalysis_comment//:go_default_library",
"@org_golang_x_tools//go/analysis:go_default_library",
"@org_golang_x_tools//go/analysis/passes/inspect:go_default_library",
"@org_golang_x_tools//go/ast/inspector:go_default_library",
],
)
go_test(
name = "go_default_test",
srcs = ["analyzer_test.go"],
data = glob(["testdata/**"]) + [
"@go_sdk//:files",
],
deps = [
":go_default_library",
"//build/bazel:go_default_library",
"@org_golang_x_tools//go/analysis/analysistest:go_default_library",
],
)

View File

@@ -0,0 +1,107 @@
package uintcast
import (
"errors"
"go/ast"
"go/types"
"strings"
"github.com/gostaticanalysis/comment"
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/analysis/passes/inspect"
"golang.org/x/tools/go/ast/inspector"
)
// Doc explaining the tool.
const Doc = "Ensure that uint variables are not cast improperly where the value could overflow. " +
"This check can be suppressed with the `lint:ignore uintcast` comment with proper justification."
// Analyzer runs static analysis.
var Analyzer = &analysis.Analyzer{
Name: "uintcast",
Doc: Doc,
Requires: []*analysis.Analyzer{inspect.Analyzer},
Run: run,
}
func run(pass *analysis.Pass) (interface{}, error) {
inspection, ok := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
if !ok {
return nil, errors.New("analyzer is not type *inspector.Inspector")
}
nodeFilter := []ast.Node{
(*ast.CallExpr)(nil),
}
commentMap := comment.New(pass.Fset, pass.Files)
inspection.Preorder(nodeFilter, func(node ast.Node) {
cg := commentMap.CommentsByPosLine(pass.Fset, node.Pos())
for _, c := range cg {
if strings.Contains(c.Text(), "lint:ignore uintcast") {
return
}
}
switch node := node.(type) {
case *ast.CallExpr:
// Cast/conversion calls have one argument and no ellipsis.
if len(node.Args) != 1 || node.Ellipsis.IsValid() {
return
}
var typ *types.Basic
switch arg := node.Args[0].(type) {
case *ast.Ident:
typ, ok = basicType(pass.TypesInfo.Types[arg].Type)
case *ast.CallExpr:
// Check if the call is a builtin conversion/anon identifier.
typ, ok = basicType(pass.TypesInfo.Types[arg].Type)
if !ok {
// Otherwise, it might be a declared function call with a return type.
typ, ok = funcReturnType(pass.TypesInfo.Types[arg.Fun].Type)
}
}
if typ == nil || !ok {
return
}
// Ignore types that are not uint variants.
if typ.Kind() < types.Uint || typ.Kind() > types.Uint64 {
return
}
if fnTyp, ok := pass.TypesInfo.Types[node.Fun].Type.(*types.Basic); ok {
if fnTyp.Kind() >= types.Int && fnTyp.Kind() <= types.Int64 {
pass.Reportf(node.Args[0].Pos(), "Unsafe cast from %s to %s.", typ, fnTyp)
}
}
}
})
return nil, nil
}
func basicType(obj types.Type) (*types.Basic, bool) {
if obj == nil {
return nil, false
}
fromTyp, ok := obj.(*types.Basic)
if !ok && obj.Underlying() != nil {
// Try to get the underlying type
fromTyp, ok = obj.Underlying().(*types.Basic)
}
return fromTyp, ok
}
func funcReturnType(obj types.Type) (*types.Basic, bool) {
if obj == nil {
return nil, false
}
fnTyp, ok := obj.(*types.Signature)
if !ok {
return nil, ok
}
return basicType(fnTyp.Results().At(0).Type())
}

View File

@@ -0,0 +1,21 @@
package uintcast_test
import (
"testing"
"github.com/prysmaticlabs/prysm/build/bazel"
"github.com/prysmaticlabs/prysm/tools/analyzers/uintcast"
"golang.org/x/tools/go/analysis/analysistest"
)
func init() {
if bazel.BuiltWithBazel() {
bazel.SetGoEnv()
}
}
func TestAnalyzer(t *testing.T) {
testdata := bazel.TestDataPath(t)
analysistest.TestData = func() string { return testdata }
analysistest.Run(t, testdata, uintcast.Analyzer)
}

View File

@@ -0,0 +1,87 @@
package testdata
import (
"math"
"time"
)
// Uint64CastToInt --
func Uint64CastToInt() {
a := uint64(math.MaxUint64)
b := int(a) // want "Unsafe cast from uint64 to int."
_ = b
}
// Uint64CastToIntIfStatement --
func Uint64CastToIntIfStatement() {
var b []string
a := uint64(math.MaxUint64)
if len(b) < int(a) { // want "Unsafe cast from uint64 to int."
return
}
}
type Slot = uint64
// BaseTypes should alert on alias like Slot.
func BaseTypes() {
var slot Slot
bad := int(slot) // want "Unsafe cast from uint64 to int."
_ = bad
}
func Uint64CastInStruct() {
type S struct {
a int
}
s := S{
a: int(uint64(5)), // want "Unsafe cast from uint64 to int."
}
_ = s
}
func Uint64CastFunctionReturn() {
fn := func() uint64 {
return 5
}
a := int(fn()) // want "Unsafe cast from uint64 to int."
_ = a
}
// IgnoredResult should not report an error.
func IgnoredResult() {
a := uint64(math.MaxUint64)
b := int(a) // lint:ignore uintcast -- test code
_ = b
}
// IgnoredIfStatement should not report an error.
func IgnoredIfStatement() {
var balances []int
var numDeposits uint64
var i int
var balance int
// lint:ignore uintcast -- test code
if len(balances) == int(numDeposits) {
balance = balances[i]
}
_ = balance
}
func IgnoreInFunctionCall() bool {
var timestamp uint64
var timeout time.Time
return time.Unix(int64(timestamp), 0).Before(timeout) // lint:ignore uintcast -- test code
}
func IgnoreWithOtherComments() bool {
var timestamp uint64
var timeout time.Time
// I plan to live forever. Maybe we should not do this?
return time.Unix(int64(timestamp), 0).Before(timeout) // lint:ignore uintcast -- timestamp will not exceed int64 in your lifetime.
}

View File

@@ -83,7 +83,7 @@ func main() {
// Construct label of each node.
rStr := hex.EncodeToString(r[:2])
label := "slot: " + strconv.Itoa(int(b.Block().Slot())) + "\n root: " + rStr
label := "slot: " + strconv.Itoa(int(b.Block().Slot())) + "\n root: " + rStr // lint:ignore uintcast -- this is OK for logging.
dotN := graph.Node(rStr).Box().Attr("label", label)
n := &node{

View File

@@ -16,6 +16,7 @@ go_library(
"//proto/prysm/v1alpha1/block:go_default_library",
"//proto/prysm/v1alpha1/wrapper:go_default_library",
"//time/slots:go_default_library",
"@com_github_prysmaticlabs_eth2_types//:go_default_library",
"@org_golang_google_grpc//:go_default_library",
"@org_golang_x_sync//errgroup:go_default_library",
],

View File

@@ -6,6 +6,7 @@ import (
"fmt"
"time"
types "github.com/prysmaticlabs/eth2-types"
"github.com/prysmaticlabs/prysm/config/params"
v1alpha1 "github.com/prysmaticlabs/prysm/proto/prysm/v1alpha1"
"github.com/prysmaticlabs/prysm/proto/prysm/v1alpha1/block"
@@ -47,7 +48,7 @@ func main() {
}
fmt.Printf("Next period starts at epoch %d (%s)\n", nextStart, time.Until(nextStartTime))
for i := 0; i < int(current.Sub(uint64(start))); i++ {
for i := types.Epoch(0); i < current.Sub(uint64(start)); i++ {
j := i
g.Go(func() error {
resp, err := c.ListBeaconBlocks(ctx, &v1alpha1.ListBlocksRequest{

View File

@@ -324,7 +324,7 @@ func printStates(stateC <-chan *modifiedState, doneC chan<- bool) {
log.Infof("---- row = %04d, slot = %8d, epoch = %8d, key = %s ----", mst.rowCount, st.Slot(), st.Slot()/params.BeaconConfig().SlotsPerEpoch, hexutils.BytesToHex(mst.key))
log.Infof("key : %s", hexutils.BytesToHex(mst.key))
log.Infof("value : compressed size = %s", humanize.Bytes(mst.valueSize))
t := time.Unix(int64(st.GenesisTime()), 0)
t := time.Unix(int64(st.GenesisTime()), 0) // lint:ignore uintcast -- Genesis time will not exceed int64 in your lifetime.
log.Infof("genesis_time : %s", t.Format(time.UnixDate))
log.Infof("genesis_validators_root : %s", hexutils.BytesToHex(st.GenesisValidatorsRoot()))
log.Infof("slot : %d", st.Slot())