From cf91a059c937a668f5a43decde20c7c39ea8d12c Mon Sep 17 00:00:00 2001 From: Emmanuel Odeke Date: Mon, 29 Jan 2018 15:09:14 -0700 Subject: [PATCH 1/2] types: Context.GetOp should never crash Ensure that requesting version <= 0 doesn't cause a runtime out of bounds dereference, with a simple validation and accompanying tests to ensure we never regress. Since GetOp allows int64, it is fair game that it should except out of range inputs, plus this is an SDK so is bound to be abused both unintentionally and intentionally. Fixes #400 --- types/context.go | 2 +- types/context_test.go | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) create mode 100644 types/context_test.go diff --git a/types/context.go b/types/context.go index a0f2c29b0f..649f15b931 100644 --- a/types/context.go +++ b/types/context.go @@ -238,7 +238,7 @@ func (pst *thePast) getOp(ver int64) (Op, bool) { pst.mtx.RLock() defer pst.mtx.RUnlock() l := int64(len(pst.ops)) - if l < ver { + if l < ver || ver <= 0 { return Op{}, false } else { return pst.ops[ver-1], true diff --git a/types/context_test.go b/types/context_test.go new file mode 100644 index 0000000000..36d8099b95 --- /dev/null +++ b/types/context_test.go @@ -0,0 +1,20 @@ +package types_test + +import ( + "testing" + + "github.com/cosmos/cosmos-sdk/types" + abci "github.com/tendermint/abci/types" +) + +func TestContextGetOpShouldNeverPanic(t *testing.T) { + var ms types.MultiStore + ctx := types.NewContext(ms, abci.Header{}, false, nil) + indices := []int64{ + -10, 1, 0, 10, 20, + } + + for _, index := range indices { + _, _ = ctx.GetOp(index) + } +} From 14ce7f3366a2502a92a38a7fb57fb0b132d0394a Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Tue, 6 Feb 2018 14:48:18 -0500 Subject: [PATCH 2/2] types: update comments on ctx.GetOp --- types/context.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/types/context.go b/types/context.go index 649f15b931..4dbf64c3e2 100644 --- a/types/context.go +++ b/types/context.go @@ -186,7 +186,7 @@ func (c Context) WithTxBytes(txBytes []byte) Context { //---------------------------------------- // thePast -// Returns false if ver > 0. +// Returns false if ver <= 0 || ver > len(c.pst.ops). // The first operation is version 1. func (c Context) GetOp(ver int64) (Op, bool) { return c.pst.getOp(ver) @@ -232,7 +232,7 @@ func (pst *thePast) version() int { return pst.ver } -// Returns false if ver > 0. +// Returns false if ver <= 0 || ver > len(pst.ops). // The first operation is version 1. func (pst *thePast) getOp(ver int64) (Op, bool) { pst.mtx.RLock()