diff --git a/x/uexecutor/types/inbound.go b/x/uexecutor/types/inbound.go index c857d48a..c9959ff1 100644 --- a/x/uexecutor/types/inbound.go +++ b/x/uexecutor/types/inbound.go @@ -3,7 +3,6 @@ package types import ( "encoding/json" "fmt" - "math/big" "strings" "cosmossdk.io/errors" @@ -128,9 +127,10 @@ func (p Inbound) ValidateForExecution() error { if strings.TrimSpace(p.Amount) == "" { return errors.Wrap(sdkerrors.ErrInvalidRequest, "amount cannot be empty") } - bi, ok := new(big.Int).SetString(p.Amount, 10) - if !ok || bi.Sign() < 0 { - return errors.Wrap(sdkerrors.ErrInvalidRequest, "amount must be a valid non-negative uint256") + // Length-capped, range-checked uint256 parse — see F-2026-18798. + bi, err := ValidateUint256String(p.Amount, "amount must be a valid non-negative uint256") + if err != nil { + return err } // Only GAS_AND_PAYLOAD and FUNDS_AND_PAYLOAD allow zero amount (skip deposit, still execute payload) if bi.Sign() == 0 && p.TxType != TxType_GAS_AND_PAYLOAD && p.TxType != TxType_FUNDS_AND_PAYLOAD { diff --git a/x/uexecutor/types/outbound_tx.go b/x/uexecutor/types/outbound_tx.go index 61ef3636..c1b8586c 100644 --- a/x/uexecutor/types/outbound_tx.go +++ b/x/uexecutor/types/outbound_tx.go @@ -57,7 +57,12 @@ func (p OutboundTx) ValidateBasic() error { if strings.TrimSpace(p.Amount) == "" { return errors.Wrap(sdkerrors.ErrInvalidRequest, "amount cannot be empty for funds tx") } - if bi, ok := new(big.Int).SetString(p.Amount, 10); !ok || bi.Sign() <= 0 { + // Length-capped, range-checked uint256 parse — see F-2026-18798. + bi, err := ValidateUint256String(p.Amount, "amount must be a valid positive uint256") + if err != nil { + return err + } + if bi.Sign() <= 0 { return errors.Wrap(sdkerrors.ErrInvalidRequest, "amount must be a valid positive uint256") } } diff --git a/x/uexecutor/types/uint256.go b/x/uexecutor/types/uint256.go new file mode 100644 index 00000000..6efe7d22 --- /dev/null +++ b/x/uexecutor/types/uint256.go @@ -0,0 +1,65 @@ +package types + +import ( + "math/big" + + "cosmossdk.io/errors" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" +) + +const ( + // MaxUint256Bits is the width of a Solidity uint256. Anything wider cannot be + // ABI-encoded faithfully: go-ethereum's encoder truncates mod 2^256 *silently*, + // so an over-range field would make the UEA execute a value different from the + // one the user signed over. + MaxUint256Bits = 256 + + // MaxUint256DecimalLen caps the decimal string length accepted for a uint256 + // field. 2^256-1 is exactly 78 digits; 80 leaves slack for clients that + // zero-pad. It is a cheap pre-filter, not the range check — see + // ValidateUint256String. + MaxUint256DecimalLen = 80 +) + +// ValidateUint256String parses value as a base-10 uint256 and returns it. +// +// The order of the three checks is load-bearing (audit finding F-2026-18798): +// +// 1. Length cap FIRST, before big.Int.SetString. big.Int decimal parsing is +// superlinear in the digit count — as reported in the finding: 78 digits +// 18µs · 100k 24.2ms · 400k 486.6ms · 900k 3.353s. This runs in +// ValidateBasic, which BaseApp executes via validateBasicTxMsgs *before* the +// ante handler, on messages that are gasless — so the work is free and +// unmetered to the attacker, and it is paid per field. Rejecting on len() +// makes that O(1) instead of O(n²). +// +// 2. Parse, rejecting non-numeric and negative input (pre-existing behaviour). +// +// 3. BitLen() <= 256. This is the authoritative range check and the one that +// closes the silent-truncation gap. The length cap alone is NOT sufficient: +// 78 nines is only 78 characters but has BitLen 260, i.e. it fits the cap +// and still overflows uint256. +// +// errMsg is the caller's message for a malformed or negative value, so each call +// site keeps its own wording; the two range failures append a specific reason. +func ValidateUint256String(value string, errMsg string) (*big.Int, error) { + // 1. Cheap reject before the expensive parse. + if len(value) > MaxUint256DecimalLen { + return nil, errors.Wrapf(sdkerrors.ErrInvalidRequest, + "%s: length %d exceeds the maximum of %d characters", errMsg, len(value), MaxUint256DecimalLen) + } + + // 2. Parse. + bi, ok := new(big.Int).SetString(value, 10) + if !ok || bi.Sign() < 0 { + return nil, errors.Wrap(sdkerrors.ErrInvalidRequest, errMsg) + } + + // 3. Authoritative uint256 range check. + if bi.BitLen() > MaxUint256Bits { + return nil, errors.Wrapf(sdkerrors.ErrInvalidRequest, + "%s: value exceeds the uint256 range", errMsg) + } + + return bi, nil +} diff --git a/x/uexecutor/types/uint256_test.go b/x/uexecutor/types/uint256_test.go new file mode 100644 index 00000000..7aade8e4 --- /dev/null +++ b/x/uexecutor/types/uint256_test.go @@ -0,0 +1,356 @@ +package types_test + +import ( + "math/big" + "strings" + "testing" + "time" + + "github.com/pushchain/push-chain-node/x/uexecutor/types" + "github.com/stretchr/testify/require" +) + +// Regression coverage for F-2026-18798 — UExecutor ValidateBasic parsed unbounded +// decimal strings before ante, and never bounded them to uint256. +// +// Two independent defects, and therefore two independent kinds of test here: +// +// - DoS: big.Int decimal parsing is superlinear, ValidateBasic runs before the +// ante handler on a gasless message, and the cost is paid per field. The +// length cap is what makes the reject O(1) — only the *timing* assertions +// below catch its removal, because BitLen still rejects the value. +// - Silent truncation: go-ethereum's ABI encoder truncates mod 2^256 without +// erroring, so an over-range value would execute an amount different from the +// one signed. Only BitLen catches that — 78 nines fits inside the 80-char cap +// but has BitLen 260. + +const ( + // 2^256-1 — the largest legal uint256, exactly 78 digits, BitLen 256. + maxUint256Dec = "115792089237316195423570985008687907853269984665640564039457584007913129639935" + // 2^256 — one past the top, BitLen 257. + overMaxUint256Dec = "115792089237316195423570985008687907853269984665640564039457584007913129639936" + // dosDigits sizes the DoS input. big.Int decimal parsing is superlinear — + // measured on the dev machine: 78 digits 24µs · 100k 9.2ms · 400k 107ms · + // 900k 532ms · 2M 2.6s · 3M 5.7s (the finding reports 3.353s at 900k on + // slower hardware). 3M is chosen so that the two margins are both wide: the + // length cap rejects it in O(1) — nanoseconds — while a parse of it overruns + // dosBudget several times over, so removing the cap fails this test loudly. + dosDigits = 3_000_000 + // dosBudget is deliberately generous relative to the ~nanoseconds an O(1) + // length reject costs, so the assertion cannot flake on a loaded CI runner, + // while still failing hard if the length cap is removed and the superlinear + // parse comes back. + dosBudget = time.Second +) + +// nines78 is 78 characters — inside the 80-char cap — but BitLen 260. This is the +// case that proves a length cap alone is not sufficient. +func nines78() string { return strings.Repeat("9", 78) } + +func hugeDecimal() string { return strings.Repeat("9", dosDigits) } + +// baseValidInbound mirrors the valid FUNDS fixture used in inbound_test.go. +func baseValidInbound() types.Inbound { + return types.Inbound{ + SourceChain: "eip155:11155111", + TxHash: "0x123abc", + Sender: "0x000000000000000000000000000000000000dead", + Recipient: "0x000000000000000000000000000000000000beef", + Amount: "1000", + AssetAddr: "0x000000000000000000000000000000000000cafe", + LogIndex: "1", + TxType: types.TxType_FUNDS, + } +} + +func TestValidateUint256String_Bounds(t *testing.T) { + tests := []struct { + name string + value string + expectError bool + errContains string + }{ + {name: "zero", value: "0"}, + {name: "small", value: "21000"}, + {name: "one wei", value: "1"}, + {name: "typical 1e18", value: "1000000000000000000"}, + {name: "zero padded within cap", value: strings.Repeat("0", 60) + "12345"}, + { + name: "max uint256 accepted", + value: maxUint256Dec, + }, + { + name: "2^256 rejected", + value: overMaxUint256Dec, + expectError: true, + errContains: "exceeds the uint256 range", + }, + { + name: "78 nines rejected despite fitting the length cap", + value: nines78(), + expectError: true, + errContains: "exceeds the uint256 range", + }, + { + name: "negative rejected", + value: "-1", + expectError: true, + errContains: "test field must be valid", + }, + { + name: "non-numeric rejected", + value: "not-a-number", + expectError: true, + errContains: "test field must be valid", + }, + { + name: "decimal point rejected", + value: "12.34", + expectError: true, + errContains: "test field must be valid", + }, + { + name: "over length cap rejected", + value: strings.Repeat("1", types.MaxUint256DecimalLen+1), + expectError: true, + errContains: "exceeds the maximum of 80 characters", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + bi, err := types.ValidateUint256String(tc.value, "test field must be valid") + + if tc.expectError { + require.Error(t, err) + require.Contains(t, err.Error(), tc.errContains) + require.Nil(t, bi) + return + } + + require.NoError(t, err) + require.NotNil(t, bi) + expected, ok := new(big.Int).SetString(tc.value, 10) + require.True(t, ok) + require.Zero(t, bi.Cmp(expected)) + }) + } +} + +// The boundary pair, stated explicitly: max uint256 in, one past it out. +func TestValidateUint256String_BitLenBoundary(t *testing.T) { + max, ok := new(big.Int).SetString(maxUint256Dec, 10) + require.True(t, ok) + require.Equal(t, 256, max.BitLen(), "sanity: max uint256 is 256 bits") + + over, ok := new(big.Int).SetString(overMaxUint256Dec, 10) + require.True(t, ok) + require.Equal(t, 257, over.BitLen(), "sanity: 2^256 is 257 bits") + + nines, ok := new(big.Int).SetString(nines78(), 10) + require.True(t, ok) + require.Equal(t, 260, nines.BitLen(), "sanity: 78 nines is 260 bits") + require.LessOrEqual(t, len(nines78()), types.MaxUint256DecimalLen, + "sanity: 78 nines fits the length cap, so only BitLen can reject it") + + _, err := types.ValidateUint256String(maxUint256Dec, "amount must be valid") + require.NoError(t, err, "2^256-1 must be accepted") + + _, err = types.ValidateUint256String(overMaxUint256Dec, "amount must be valid") + require.Error(t, err, "2^256 must be rejected") + + _, err = types.ValidateUint256String(nines78(), "amount must be valid") + require.Error(t, err, "78 nines must be rejected") +} + +// F-2026-18798, DoS half. A multi-million-digit field must be rejected, and +// rejected fast. The timing bound is asserted first and on purpose: it is the only +// assertion that fails if the length cap is dropped, because BitLen still rejects +// the value — just after paying for the parse. +func TestUniversalPayload_ValidateBasic_RejectsHugeDecimalFast(t *testing.T) { + huge := hugeDecimal() + + // Every numeric field is reachable, and in the real message the attacker pays + // for none of them — ValidateBasic runs before ante on a gasless msg. + fields := []struct { + name string + payload types.UniversalPayload + }{ + {"value", types.UniversalPayload{To: mockHexAddress(), Value: huge}}, + {"gas_limit", types.UniversalPayload{To: mockHexAddress(), GasLimit: huge}}, + {"max_fee_per_gas", types.UniversalPayload{To: mockHexAddress(), MaxFeePerGas: huge}}, + {"max_priority_fee_per_gas", types.UniversalPayload{To: mockHexAddress(), MaxPriorityFeePerGas: huge}}, + {"nonce", types.UniversalPayload{To: mockHexAddress(), Nonce: huge}}, + {"deadline", types.UniversalPayload{To: mockHexAddress(), Deadline: huge}}, + } + + for _, f := range fields { + t.Run(f.name, func(t *testing.T) { + start := time.Now() + err := f.payload.ValidateBasic() + elapsed := time.Since(start) + + require.Error(t, err, "%s: a %d-digit value must be rejected", f.name, dosDigits) + require.Less(t, elapsed, dosBudget, + "%s: rejecting a %d-digit value took %s — the length cap must reject before big.Int parses", + f.name, dosDigits, elapsed) + require.Contains(t, err.Error(), "exceeds the maximum of 80 characters", + "%s: must be rejected on length, before the parse", f.name) + }) + } +} + +// Same DoS shape at the other two call sites. +func TestInboundAndOutbound_RejectHugeDecimalFast(t *testing.T) { + huge := hugeDecimal() + + t.Run("inbound amount", func(t *testing.T) { + ib := baseValidInbound() + ib.Amount = huge + + start := time.Now() + err := ib.ValidateForExecution() + elapsed := time.Since(start) + + require.Error(t, err) + require.Less(t, elapsed, dosBudget, "rejecting a %d-digit amount took %s", dosDigits, elapsed) + require.Contains(t, err.Error(), "exceeds the maximum of 80 characters") + }) + + t.Run("outbound amount", func(t *testing.T) { + ob := baseValidOutbound() + ob.Amount = huge + + start := time.Now() + err := ob.ValidateBasic() + elapsed := time.Since(start) + + require.Error(t, err) + require.Less(t, elapsed, dosBudget, "rejecting a %d-digit amount took %s", dosDigits, elapsed) + require.Contains(t, err.Error(), "exceeds the maximum of 80 characters") + }) +} + +// F-2026-18798, truncation half, per call site. go-ethereum packs an over-range +// value mod 2^256 without erroring, so these must never reach the encoder. +func TestUniversalPayload_ValidateBasic_Uint256Range(t *testing.T) { + tests := []struct { + name string + payload types.UniversalPayload + expectError bool + }{ + { + name: "max uint256 value accepted", + payload: types.UniversalPayload{To: mockHexAddress(), Value: maxUint256Dec}, + }, + { + name: "max uint256 on every field accepted", + payload: types.UniversalPayload{ + To: mockHexAddress(), + Value: maxUint256Dec, + GasLimit: maxUint256Dec, + MaxFeePerGas: maxUint256Dec, + MaxPriorityFeePerGas: maxUint256Dec, + Nonce: maxUint256Dec, + Deadline: maxUint256Dec, + }, + }, + { + name: "empty numeric fields still skipped", + payload: types.UniversalPayload{To: mockHexAddress()}, + }, + { + name: "2^256 value rejected", + payload: types.UniversalPayload{To: mockHexAddress(), Value: overMaxUint256Dec}, + expectError: true, + }, + { + name: "78 nines value rejected", + payload: types.UniversalPayload{To: mockHexAddress(), Value: nines78()}, + expectError: true, + }, + { + name: "78 nines gas_limit rejected", + payload: types.UniversalPayload{To: mockHexAddress(), GasLimit: nines78()}, + expectError: true, + }, + { + name: "78 nines nonce rejected", + payload: types.UniversalPayload{To: mockHexAddress(), Nonce: nines78()}, + expectError: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + err := tc.payload.ValidateBasic() + if tc.expectError { + require.Error(t, err) + require.Contains(t, err.Error(), "exceeds the uint256 range") + } else { + require.NoError(t, err) + } + }) + } +} + +func TestInbound_ValidateForExecution_Uint256Range(t *testing.T) { + tests := []struct { + name string + amount string + expectError bool + }{ + {name: "normal amount accepted", amount: "1000"}, + {name: "max uint256 accepted", amount: maxUint256Dec}, + {name: "2^256 rejected", amount: overMaxUint256Dec, expectError: true}, + {name: "78 nines rejected", amount: nines78(), expectError: true}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + ib := baseValidInbound() + ib.Amount = tc.amount + + err := ib.ValidateForExecution() + if tc.expectError { + require.Error(t, err) + require.Contains(t, err.Error(), "exceeds the uint256 range") + } else { + require.NoError(t, err) + } + }) + } +} + +func TestOutboundTx_ValidateBasic_Uint256Range(t *testing.T) { + tests := []struct { + name string + amount string + expectError bool + errContains string + }{ + {name: "normal amount accepted", amount: "1000"}, + {name: "max uint256 accepted", amount: maxUint256Dec}, + {name: "2^256 rejected", amount: overMaxUint256Dec, expectError: true, errContains: "exceeds the uint256 range"}, + {name: "78 nines rejected", amount: nines78(), expectError: true, errContains: "exceeds the uint256 range"}, + // Pre-existing semantics preserved: this site requires strictly positive. + {name: "zero still rejected", amount: "0", expectError: true, errContains: "amount must be a valid positive uint256"}, + {name: "negative still rejected", amount: "-1", expectError: true, errContains: "amount must be a valid positive uint256"}, + {name: "non-numeric still rejected", amount: "abc", expectError: true, errContains: "amount must be a valid positive uint256"}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + ob := baseValidOutbound() + ob.Amount = tc.amount + + err := ob.ValidateBasic() + if tc.expectError { + require.Error(t, err) + require.Contains(t, err.Error(), tc.errContains) + } else { + require.NoError(t, err) + } + }) + } +} diff --git a/x/uexecutor/types/universal_payload.go b/x/uexecutor/types/universal_payload.go index 500f8acb..cbab3c88 100644 --- a/x/uexecutor/types/universal_payload.go +++ b/x/uexecutor/types/universal_payload.go @@ -3,7 +3,6 @@ package types import ( "encoding/hex" "encoding/json" - "math/big" "strings" "cosmossdk.io/errors" @@ -50,9 +49,9 @@ func (p UniversalPayload) ValidateBasic() error { for fieldName, value := range uintFields { if value != "" { - bi, ok := new(big.Int).SetString(value, 10) - if !ok || bi.Sign() < 0 { - return errors.Wrapf(sdkerrors.ErrInvalidRequest, "%s must be a valid unsigned integer", fieldName) + // Length-capped, range-checked uint256 parse — see F-2026-18798. + if _, err := ValidateUint256String(value, fieldName+" must be a valid unsigned integer"); err != nil { + return err } } }