gosec icon indicating copy to clipboard operation
gosec copied to clipboard

rules/sdk: add check for missing .IsNil check before deference after being cast from an interface{} to avoid nil pointer dereferences

Open odeke-em opened this issue 3 years ago • 1 comments

Summary

There is this cosmos-sdk bug https://github.com/cosmos/cosmos-sdk/issues/5621 in which an sdk.Dec value was cast from an interface{} value, thus can be nil. We really should be able to detect objects with a .IsNil and if we didn't invoke that and invoke any other method, we should report that

Steps to reproduce the behavior

package main

import "github.com/cosmos/cosmos-sdk/types"

func main() {
	var n95 types.Dec
	_ = n95.Abs()
}
$ go run it.go 
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x10 pc=0x1713e9c]

goroutine 1 [running]:
math/big.(*Int).Set(...)
	/Users/emmanuelodeke/go/src/go.googlesource.com/go/src/math/big/int.go:74
math/big.(*Int).Abs(...)
	/Users/emmanuelodeke/go/src/go.googlesource.com/go/src/math/big/int.go:102
github.com/cosmos/cosmos-sdk/types.Dec.Abs(...)
	/Users/emmanuelodeke/go/src/github.com/cosmos/cosmos-sdk/types/decimal.go:217
main.main()
	/Users/emmanuelodeke/go/src/github.com/cosmos/cosmos-sdk/tests/nilcheck/it.go:7 +0x1c
exit status 2

Expected behavior

That code should have been flagged by gosec. Kindly cc-ing @kirbyquerby

odeke-em avatar Jun 10 '22 05:06 odeke-em

@kirbyquerby we could even purposefully just detect the pattern where a .(types.Dec) was cast from an interface{} value and from that ensure that firstly .IsNil() was invoked.

odeke-em avatar Jun 10 '22 05:06 odeke-em