From 54d764b9a871178c155181ca4cc01769a13bb44a Mon Sep 17 00:00:00 2001
From: Emmanuel T Odeke <emmanuel@orijtech.com>
Date: Thu, 19 May 2022 23:28:06 +0300
Subject: [PATCH] fix: math: fix Uint.Unmarshal's lack of negative value
 checking (#11996)

This change adds a negative value check to Uint.Unmarshal,
which coincidentally is fixed by refactoring for code reuse.
While here, added tests to ensure we don't regress.

Fixes #11995
---
 math/uint.go      | 12 +++++-------
 math/uint_test.go | 38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+), 7 deletions(-)

diff --git a/math/uint.go b/math/uint.go
index 505984d198..02efb9fc8d 100644
--- a/math/uint.go
+++ b/math/uint.go
@@ -190,11 +190,8 @@ func (u *Uint) Unmarshal(data []byte) error {
 		return err
 	}
 
-	if u.i.BitLen() > MaxBitLen {
-		return fmt.Errorf("integer out of range; got: %d, max: %d", u.i.BitLen(), MaxBitLen)
-	}
-
-	return nil
+	// Finally check for overflow.
+	return UintOverflow(u.i)
 }
 
 // Size implements the gogo proto custom type interface.
@@ -213,8 +210,9 @@ func UintOverflow(i *big.Int) error {
 	if i.Sign() < 0 {
 		return errors.New("non-positive integer")
 	}
-	if i.BitLen() > 256 {
-		return fmt.Errorf("bit length %d greater than 256", i.BitLen())
+
+	if g, w := i.BitLen(), MaxBitLen; g > w {
+		return fmt.Errorf("integer out of range; got: %d, max: %d", g, w)
 	}
 	return nil
 }
diff --git a/math/uint_test.go b/math/uint_test.go
index bb177eb6ea..3077e50941 100644
--- a/math/uint_test.go
+++ b/math/uint_test.go
@@ -5,6 +5,7 @@ import (
 	"math"
 	"math/big"
 	"math/rand"
+	"strings"
 	"testing"
 
 	sdkmath "cosmossdk.io/math"
@@ -300,6 +301,7 @@ func TestRoundTripMarshalToUint(t *testing.T) {
 		1<<63 - 1,
 		1<<32 - 7,
 		1<<22 - 8,
+		math.MaxUint64,
 	}
 
 	for _, value := range values {
@@ -323,3 +325,39 @@ func TestRoundTripMarshalToUint(t *testing.T) {
 		})
 	}
 }
+
+func TestWeakUnmarshalNegativeSign(t *testing.T) {
+	neg10, _ := new(big.Int).SetString("-10", 0)
+	blob, err := neg10.MarshalText()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	ui := new(sdkmath.Uint)
+	err = ui.Unmarshal(blob)
+	if err == nil {
+		t.Fatal("Failed to catch the negative value")
+	}
+	if errStr := err.Error(); !strings.Contains(errStr, "non-positive") {
+		t.Fatalf("negative value not reported, got instead %q", errStr)
+	}
+}
+
+func TestWeakUnmarshalOverflow(t *testing.T) {
+	exp := new(big.Int).SetUint64(256)
+	pos10, _ := new(big.Int).SetString("10", 0)
+	exp10Pow256 := new(big.Int).Exp(pos10, exp, nil)
+	blob, err := exp10Pow256.MarshalText()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	ui := new(sdkmath.Uint)
+	err = ui.Unmarshal(blob)
+	if err == nil {
+		t.Fatal("Failed to catch the overflowed value")
+	}
+	if errStr := err.Error(); !strings.Contains(errStr, "out of range") {
+		t.Fatalf("out of range value not reported, got instead %q", errStr)
+	}
+}