x/interchainstaking/types: DetermineAllocationsForUndelegation should defensively guard that lookups from availablePerValidator map could returns an empty coin which could panic when compared against

[odeke-em]

Summary of Bug

Found through fuzzing that given these inputs

• []byte("{"Cur_AlloCs":{"":"0"},"Amount":[{"Amount":"1"}]}")
• []byte("{"Cur_AlloCs":{"":"1"},"Amount":[{"Amount":"1"}]}")

in this code repro below

package types_test

import (
	"encoding/json"
	"testing"
)

type detRedemptionTest struct {
	Name               string                 `json:"name"`
	CurrentAllocations map[string]sdkmath.Int `json:"cur_allocs"`
	Unlocked           map[string]sdkmath.Int `json:"Unlocked"`
	TargetAllocations  types.ValidatorIntents `json:"targ_allocs"`
	Amount             sdk.Coins              `json:"amount"`
	expected           map[string]sdkmath.Int
}

func TestDetermineAllocationsForUndelegationRepro(f *testing.F) {
	inputJSONs := []string{
		"{\"Cur_AlloCs\":{\"\":\"0\"},\"Amount\":[{\"Amount\":\"1\"}]}",
		"{\"Cur_AlloCs\":{\"\":\"1\"},\"Amount\":[{\"Amount\":\"1\"}]}",
	}
	for _, str := range inputJSONs {
		inputJSON := []byte(str)
		drt := new(detRedemptionTest)
		if err := json.Unmarshal(inputJSON, drt); err != nil {
			return
		}
		_, _ = types.DetermineAllocationsForUndelegation(
			drt.CurrentAllocations,
			map[string]bool{},
			sum(drt.CurrentAllocations),
			drt.TargetAllocations,
			drt.Unlocked,
			drt.Amount)
	}
}

Panics

• Panic given "{\"Cur_AlloCs\":{\"\":\"0\"},\"Amount\":[{\"Amount\":\"1\"}]}"

        testing.go:1590: panic: runtime error: invalid memory address or nil pointer dereference
            goroutine 6644 [running]:
            runtime/debug.Stack()
            	/Users/emmanuelodeke/go/src/go.googlesource.com/go/src/runtime/debug/stack.go:26 +0x9b
            testing.tRunner.func1()
            	/Users/emmanuelodeke/go/src/go.googlesource.com/go/src/testing/testing.go:1590 +0x1c8
            panic({0x111165d20?, 0x11258abc0?})
            	/Users/emmanuelodeke/go/src/go.googlesource.com/go/src/runtime/panic.go:759 +0x132
            math/big.(*Int).Cmp(0xc00a1e48e0?, 0xc00a1dcb70?)
            	/Users/emmanuelodeke/go/src/go.googlesource.com/go/src/math/big/int.go:381 +0x60
            cosmossdk.io/math.lte(...)
            	/Users/emmanuelodeke/go/pkg/mod/cosmossdk.io/[email protected]/int.go:44
            cosmossdk.io/math.Int.LTE(...)
            	/Users/emmanuelodeke/go/pkg/mod/cosmossdk.io/[email protected]/int.go:273
            github.com/quicksilver-zone/quicksilver/x/interchainstaking/types.DetermineAllocationsForUndelegation(0xc00a1c5140, 0xc00121d7f0, {0x0?}, {0x0, 0x0, 0x0}, 0x0, {0xc00a1dca68, 0x1, 0x1})
            	/Users/emmanuelodeke/go/src/github.com/quicksilver-zone/quicksilver/x/interchainstaking/types/redemptions.go:173 +0x2585
            github.com/quicksilver-zone/quicksilver/x/interchainstaking/types_test.FuzzDetermineAllocationsForUndelegation.func1(0x0?, {0xc00122b700, 0x31, 0x580})
            	/Users/emmanuelodeke/go/src/github.com/quicksilver-zone/quicksilver/x/interchainstaking/types/fuzz_test.go:70 +0x131
            reflect.Value.call({0x1110ddf00?, 0x11142f9f8?, 0x13?}, {0x110632997, 0x4}, {0xc00a1c50e0, 0x2, 0x2?})
            	/Users/emmanuelodeke/go/src/go.googlesource.com/go/src/reflect/value.go:581 +0xca6
            reflect.Value.Call({0x1110ddf00?, 0x11142f9f8?, 0x10e1b424d?}, {0xc00a1c50e0?, 0x11142c380?, 0xf?})
            	/Users/emmanuelodeke/go/src/go.googlesource.com/go/src/reflect/value.go:365 +0xb9
            testing.(*F).Fuzz.func1.1(0xc00a1f4d00?)
            	/Users/emmanuelodeke/go/src/go.googlesource.com/go/src/testing/fuzz.go:335 +0x305
            testing.tRunner(0xc00a1f4d00, 0xc00a1f6360)
            	/Users/emmanuelodeke/go/src/go.googlesource.com/go/src/testing/testing.go:1689 +0xf4
            created by testing.(*F).Fuzz.func1 in goroutine 37
            	/Users/emmanuelodeke/go/src/go.googlesource.com/go/src/testing/fuzz.go:322 +0x577

• Panic given "{\"Cur_AlloCs\":{\"\":\"1\"},\"Amount\":[{\"Amount\":\"1\"}]}"

        testing.go:1590: panic: runtime error: invalid memory address or nil pointer dereference
            goroutine 6650 [running]:
            runtime/debug.Stack()
            	/Users/emmanuelodeke/go/src/go.googlesource.com/go/src/runtime/debug/stack.go:26 +0x9b
            testing.tRunner.func1()
            	/Users/emmanuelodeke/go/src/go.googlesource.com/go/src/testing/testing.go:1590 +0x1c8
            panic({0x104acbd20?, 0x105ef0bc0?})
            	/Users/emmanuelodeke/go/src/go.googlesource.com/go/src/runtime/panic.go:759 +0x132
            math/big.(*Int).Cmp(0x104ab1280?, 0xc009faf620?)
            	/Users/emmanuelodeke/go/src/go.googlesource.com/go/src/math/big/int.go:381 +0x60
            cosmossdk.io/math.gt(...)
            	/Users/emmanuelodeke/go/pkg/mod/cosmossdk.io/[email protected]/int.go:38
            cosmossdk.io/math.Int.GT(...)
            	/Users/emmanuelodeke/go/pkg/mod/cosmossdk.io/[email protected]/int.go:257
            github.com/quicksilver-zone/quicksilver/x/interchainstaking/types.DetermineAllocationsForUndelegation(0xc009faf590, 0xc0007d57f0, {0x0?}, {0x0, 0x0, 0x0}, 0x0, {0xc009fb3968, 0x1, 0x1})
            	/Users/emmanuelodeke/go/src/github.com/quicksilver-zone/quicksilver/x/interchainstaking/types/redemptions.go:55 +0x7e5
            github.com/quicksilver-zone/quicksilver/x/interchainstaking/types_test.FuzzDetermineAllocationsForUndelegation.func1(0x0?, {0xc001307180, 0x31, 0x580})
            	/Users/emmanuelodeke/go/src/github.com/quicksilver-zone/quicksilver/x/interchainstaking/types/fuzz_test.go:70 +0x131
            reflect.Value.call({0x104a43f00?, 0x104d959f8?, 0x13?}, {0x103f98977, 0x4}, {0xc009faf530, 0x2, 0x2?})
            	/Users/emmanuelodeke/go/src/go.googlesource.com/go/src/reflect/value.go:581 +0xca6
            reflect.Value.Call({0x104a43f00?, 0x104d959f8?, 0x101b1a1ed?}, {0xc009faf530?, 0x104d92380?, 0xf?})
            	/Users/emmanuelodeke/go/src/go.googlesource.com/go/src/reflect/value.go:365 +0xb9
            testing.(*F).Fuzz.func1.1(0xc009fe1ba0?)
            	/Users/emmanuelodeke/go/src/go.googlesource.com/go/src/testing/fuzz.go:335 +0x305
            testing.tRunner(0xc009fe1ba0, 0xc009fe4cf0)
            	/Users/emmanuelodeke/go/src/go.googlesource.com/go/src/testing/testing.go:1689 +0xf4
            created by testing.(*F).Fuzz.func1 in goroutine 21
            	/Users/emmanuelodeke/go/src/go.googlesource.com/go/src/testing/fuzz.go:322 +0x577

Version

Remedies

diff --git a/x/interchainstaking/types/redemptions.go b/x/interchainstaking/types/redemptions.go
index ec985b02..5e8ec462 100644
--- a/x/interchainstaking/types/redemptions.go
+++ b/x/interchainstaking/types/redemptions.go
@@ -52,7 +52,10 @@ func DetermineAllocationsForUndelegation(currentAllocations map[string]math.Int,
 		for idx := range overAllocated {
 			// use Amount+1 in the line below to avoid 1 remaining where truncation leaves 1 remaining - e.g. 1000 => 333/333/333 + 1.
 			outWeights[overAllocated[idx].ValoperAddress] = sdk.NewDecFromInt(overAllocated[idx].Amount).Quo(sdk.NewDecFromInt(sum)).Mul(sdk.NewDecFromInt(overAllocationSplit)).TruncateInt()
-			if outWeights[overAllocated[idx].ValoperAddress].GT(availablePerValidator[overAllocated[idx].ValoperAddress]) {
+			overAlloc := availablePerValidator[overAllocated[idx].ValoperAddress]
+			if overAlloc.IsNil() {
+				availablePerValidator[overAllocated[idx].ValoperAddress] = sdk.ZeroInt()
+			} else if outWeights[overAllocated[idx].ValoperAddress].GT(overAlloc) {
 				// use up all of overAllocated[idx] and set available to zero.
 				outWeights[overAllocated[idx].ValoperAddress] = availablePerValidator[overAllocated[idx].ValoperAddress]
 				availablePerValidator[overAllocated[idx].ValoperAddress] = sdk.ZeroInt()
@@ -170,7 +173,11 @@ func DetermineAllocationsForUndelegation(currentAllocations map[string]math.Int,
 	// the delta calculations on the next run.
 	dust := amount[0].Amount.Sub(outSum)
 	for idx := 0; idx <= len(deltas)-1; idx++ {
-		if dust.LTE(availablePerValidator[deltas[idx].ValoperAddress]) {
+		gotForValoper := availablePerValidator[deltas[idx].ValoperAddress]
+		if gotForValoper.IsNil() {
+			continue
+		}
+		if dust.LTE(gotForValoper) {
 			outWeights[deltas[idx].ValoperAddress] = outWeights[deltas[idx].ValoperAddress].Add(dust)
 			break
 		}

/cc @faddat @tropicaldog @joe-bowman

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged/assigned