Merge remote-tracking branch 'origin/audit-fixes' into merge-339

Author: 0xNilesh

app/README.md

@@ -150,7 +150,9 @@ Push Chain ships exactly one custom precompile:
 |---|---|---|
 | `0xEC00000000000000000000000000000000000001` | `usigverifier` | Ed25519 signature verification (Solana signatures over `bytes32` digests), registered at the reserved Push range |
 
-Gas cost: `4000` per `verifyEd25519` call. See [`precompiles/usigverifier/README.md`](../precompiles/usigverifier/README.md).
+Gas cost: `4000` per `verifyEd25519` call (fixed 32-byte digest), and `4000` plus `12` per 32-byte
+word of `message` for `verifyEd25519RawMessage`, whose message is hard-capped at 128 KiB. See
+[`precompiles/usigverifier/README.md`](../precompiles/usigverifier/README.md).
 
 The baseline EVM precompiles (`bech32`, `p256`, `staking`, `distribution`, `ics20`, `bank`, `gov`, `slashing`, `evidence`) are wired in via `app/precompiles.go:NewAvailableStaticPrecompiles`.
 

app/app.go

@@ -504,8 +504,31 @@ func NewChainApp(
 		logger,
 	)
 
-	// enable sign mode textual by overwriting the default tx config (after setting the bank keeper)
-	enabledSignModes := append(tx.DefaultSignModes, signingtype.SignMode_SIGN_MODE_TEXTUAL)
+	// Enabled sign modes, listed explicitly rather than appending to
+	// tx.DefaultSignModes so that what the chain accepts is stated here rather
+	// than inherited.
+	//
+	// SIGN_MODE_DIRECT_AUX is deliberately excluded (F-2026-18784). The handler
+	// in cosmossdk.io/x/tx rejects a fee payer who also signs with DIRECT_AUX
+	// using a raw string compare:
+	//
+	//	if feePayer == signerData.Address { ... unauthorized ... }
+	//
+	// BIP-173 permits an all-uppercase bech32 encoding of the same account, so
+	// an uppercase Fee.Payer aliasing the victim's lowercase signer address
+	// fails that check open, while everything downstream decodes both to the
+	// same AccAddress and deduplicates signers. A sponsor holding a victim's
+	// DIRECT_AUX signature over a fixed TxBody could then rewrite AuthInfo to
+	// charge the victim. Still present in our pinned x/tx v0.14.0.
+	//
+	// Nothing on Push signs with DIRECT_AUX — the universal client pins
+	// SIGN_MODE_DIRECT — so enabling it only exposes surface. Restore it once
+	// x/tx compares decoded bytes (or folds case), not before.
+	enabledSignModes := []signingtype.SignMode{
+		signingtype.SignMode_SIGN_MODE_DIRECT,
+		signingtype.SignMode_SIGN_MODE_LEGACY_AMINO_JSON,
+		signingtype.SignMode_SIGN_MODE_TEXTUAL,
+	}
 	txConfigOpts := tx.ConfigOptions{
 		EnabledSignModes:           enabledSignModes,
 		TextualCoinMetadataQueryFn: txmodule.NewBankKeeperCoinMetadataQueryFn(app.BankKeeper),

app/sign_modes_test.go

@@ -0,0 +1,75 @@
+package app
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	signingtype "cosmossdk.io/api/cosmos/tx/signing/v1beta1"
+	"github.com/cosmos/cosmos-sdk/x/auth/tx"
+)
+
+// Regression test for F-2026-18784 (uppercase bech32 fee payer bypasses the
+// DIRECT_AUX fee-payer guard).
+//
+// The handler in cosmossdk.io/x/tx compares the fee payer against the signer
+// with a raw string compare:
+//
+//	if feePayer == signerData.Address { ... unauthorized ... }
+//
+// BIP-173 allows an all-uppercase bech32 encoding of the same account, so an
+// uppercase Fee.Payer aliasing the victim's lowercase signer address slips past
+// that check while everything downstream decodes both to the same AccAddress.
+// Our pinned x/tx v0.14.0 still has the raw compare, so the mode stays off.
+//
+// This test exists so that a future refactor cannot quietly re-enable DIRECT_AUX
+// by going back to appending to tx.DefaultSignModes, which contains it.
+func TestEnabledSignModes_ExcludesDirectAux(t *testing.T) {
+	// setup() constructs the app without InitChain, which is all this needs.
+	// Setup() is avoided on purpose: it passes the "testing" chain ID and panics
+	// in the EVM configurator unless another test has already initialised it.
+	gapp, _ := setup(t, ChainID, false, 0)
+	modes := gapp.TxConfig().SignModeHandler().SupportedModes()
+
+	for _, m := range modes {
+		require.NotEqual(t, signingtype.SignMode_SIGN_MODE_DIRECT_AUX, m,
+			"SIGN_MODE_DIRECT_AUX must stay disabled until x/tx compares decoded "+
+				"bytes rather than raw strings (F-2026-18784)")
+	}
+}
+
+// TestEnabledSignModes_KeepsTheModesWeActuallyUse guards the other direction:
+// dropping DIRECT_AUX must not take anything else with it. The universal client
+// signs with SIGN_MODE_DIRECT, and TEXTUAL is enabled deliberately (it is not in
+// tx.DefaultSignModes and needs the bank keeper).
+func TestEnabledSignModes_KeepsTheModesWeActuallyUse(t *testing.T) {
+	gapp, _ := setup(t, ChainID, false, 0)
+	modes := gapp.TxConfig().SignModeHandler().SupportedModes()
+
+	has := func(want signingtype.SignMode) bool {
+		for _, m := range modes {
+			if m == want {
+				return true
+			}
+		}
+		return false
+	}
+
+	require.True(t, has(signingtype.SignMode_SIGN_MODE_DIRECT), "DIRECT is what the universal client signs with")
+	require.True(t, has(signingtype.SignMode_SIGN_MODE_LEGACY_AMINO_JSON), "AMINO_JSON is needed for ledger/legacy clients")
+	require.True(t, has(signingtype.SignMode_SIGN_MODE_TEXTUAL), "TEXTUAL is enabled deliberately")
+}
+
+// TestDefaultSignModesStillContainsDirectAux documents why the explicit list
+// exists. If upstream ever drops DIRECT_AUX from DefaultSignModes this test
+// fails, and the explicit enumeration can be reconsidered.
+func TestDefaultSignModesStillContainsDirectAux(t *testing.T) {
+	found := false
+	for _, m := range tx.DefaultSignModes {
+		if m.String() == "SIGN_MODE_DIRECT_AUX" {
+			found = true
+		}
+	}
+	require.True(t, found,
+		"tx.DefaultSignModes no longer contains DIRECT_AUX; the explicit list in app.go may no longer be needed")
+}

app/txpolicy/gasless.go

@@ -33,6 +33,11 @@ func IsGaslessTx(tx sdk.Tx) bool {
 	for _, msg := range msgs {
 		switch m := msg.(type) {
 		case *authz.MsgExec:
+			// An empty nest would pass the loop below vacuously and make the whole
+			// tx gasless, bypassing the fee and min-gas-price decorators (F-2026-18816).
+			if len(m.Msgs) == 0 {
+				return false
+			}
 			// Only gasless if ALL inner messages are allowed
 			for _, innerMsg := range m.Msgs {
 				if !slices.Contains(GaslessMsgTypes, innerMsg.TypeUrl) {

app/txpolicy/gasless_test.go

@@ -52,3 +52,87 @@ func TestGaslessMsgTypesExcludeEthereumTx(t *testing.T) {
 		require.True(t, txpolicy.IsGaslessTx(tx))
 	})
 }
+
+// TestIsGaslessTxAuthzExecNesting guards the authz.MsgExec branch of IsGaslessTx.
+//
+// The inner-message loop is an "all must be allowlisted" check, so an empty nest
+// satisfies it vacuously and would make the whole tx gasless - skipping
+// DeductFeeDecorator and MinGasPriceDecorator for a zero-fee tx, and handing the
+// signer a free on-chain account via AccountInitDecorator, which gates on this
+// same predicate. Nothing upstream catches it: authz.MsgExec has no ValidateBasic
+// in SDK v0.53.7, and the empty check lives only in the msg server, which runs
+// after the fee decorators (F-2026-18816).
+func TestIsGaslessTxAuthzExecNesting(t *testing.T) {
+	anyOf := func(t *testing.T, msg sdk.Msg) *codectypes.Any {
+		t.Helper()
+		a, err := codectypes.NewAnyWithValue(msg)
+		require.NoError(t, err)
+		return a
+	}
+
+	tests := []struct {
+		name    string
+		inner   []sdk.Msg
+		gasless bool
+		reason  string
+	}{
+		{
+			name:    "empty nest is not gasless",
+			inner:   nil,
+			gasless: false,
+			reason:  "an empty authz.MsgExec must not pass the inner allowlist loop vacuously",
+		},
+		{
+			name:    "empty non-nil nest is not gasless",
+			inner:   []sdk.Msg{},
+			gasless: false,
+			reason:  "a zero-length (but non-nil) inner message list must be rejected too",
+		},
+		{
+			name:    "all-allowlisted nest stays gasless",
+			inner:   []sdk.Msg{&uexecutortypes.MsgVoteInbound{}, &uexecutortypes.MsgVoteOutbound{}},
+			gasless: true,
+			reason:  "a nest of only allowlisted messages must remain gasless",
+		},
+		{
+			name:    "mixed nest is not gasless",
+			inner:   []sdk.Msg{&uexecutortypes.MsgVoteInbound{}, &evmtypes.MsgEthereumTx{}},
+			gasless: false,
+			reason:  "one non-allowlisted inner message must disqualify the whole tx",
+		},
+		{
+			name:    "nested MsgExec is not gasless",
+			inner:   []sdk.Msg{&authz.MsgExec{Msgs: []*codectypes.Any{}}},
+			gasless: false,
+			reason:  "authz.MsgExec is not itself an allowlisted type, so nesting one must not recurse into a vacuous pass",
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			// Preserve the nil vs. zero-length distinction: len() treats them the
+			// same, but constructing both proves the guard does not depend on it.
+			var inner []*codectypes.Any
+			if tc.inner != nil {
+				inner = make([]*codectypes.Any, 0, len(tc.inner))
+				for _, m := range tc.inner {
+					inner = append(inner, anyOf(t, m))
+				}
+			}
+
+			tx := msgsOnlyTx{msgs: []sdk.Msg{&authz.MsgExec{Msgs: inner}}}
+			require.Equal(t, tc.gasless, txpolicy.IsGaslessTx(tx), tc.reason)
+		})
+	}
+}
+
+// TestIsGaslessTxEmptyExecAlongsideAllowedMsg pins the multi-message case: the
+// outer loop must not let an allowlisted sibling carry an empty nest through.
+func TestIsGaslessTxEmptyExecAlongsideAllowedMsg(t *testing.T) {
+	tx := msgsOnlyTx{msgs: []sdk.Msg{
+		&uexecutortypes.MsgVoteInbound{},
+		&authz.MsgExec{},
+	}}
+	require.False(t, txpolicy.IsGaslessTx(tx),
+		"an empty authz.MsgExec must disqualify the tx even next to an allowlisted message")
+}

precompiles/usigverifier/README.md

@@ -40,11 +40,36 @@ interface IUSigVerifier {
 
 | Method | Signed bytes | Gas | Use when |
 |---|---|---|---|
-| `verifyEd25519(bytes,bytes32,bytes)` | `"0x" + hex(msgDigest)` (66 ASCII bytes) | 4000 | UEA_SVM / Solana-wallet flows where the user signs a hex string in Phantom/Solflare |
-| `verifyEd25519RawMessage(bytes,bytes,bytes)` | Raw `message` bytes | 4000 | New integrations / relayers using standard `ed25519.Sign(privKey, rawBytes)` |
+| `verifyEd25519(bytes,bytes32,bytes)` | `"0x" + hex(msgDigest)` (66 ASCII bytes) | 4000 (flat) | UEA_SVM / Solana-wallet flows where the user signs a hex string in Phantom/Solflare |
+| `verifyEd25519RawMessage(bytes,bytes,bytes)` | Raw `message` bytes | `4000 + 12` per 32-byte word of `message` | New integrations / relayers using standard `ed25519.Sign(privKey, rawBytes)` |
 
 Both methods are `view` and touch no chain state.
 
+### Why only the raw method scales with size
+
+`ed25519.Verify` hashes the whole message, so its CPU cost grows with the message
+(~58 µs at 32 B, ~146 µs at 128 KiB, ~922 µs at 1 MB). `verifyEd25519` always verifies
+the same 66-byte ASCII string no matter what the caller sends, so its cost is constant
+and its price stays flat. `verifyEd25519RawMessage` verifies caller-supplied bytes, so
+it is priced per 32-byte word — the same per-word rate the EVM `SHA-256` precompile
+charges for comparable hashing work.
+
+Because both methods are `view`, a contract can park one large message in memory and
+loop `STATICCALL`s over it, paying the calldata only once. Pricing alone is therefore
+not the whole defence: `message` is also **hard-capped at 128 KiB**
+(`MaxEd25519MessageBytes`), and anything larger reverts with `message too large`
+instead of being verified.
+
+| `len(message)` | Gas |
+|---|---|
+| 0 | 4,000 |
+| 32 B | 4,012 |
+| 1 KiB | 4,384 |
+| 8 KiB | 7,072 |
+| 64 KiB | 28,576 |
+| 128 KiB (cap) | 53,152 |
+| > 128 KiB | reverts |
+
 ## Verification Semantics
 
 Two methods, two distinct signing conventions. **A signature produced for one method will not verify under the other** — the test vectors in `query_test.go` lock this in.
@@ -69,13 +94,14 @@ Standard Ed25519 verification — signature is checked against the raw `message`
 ok = ed25519.Verify(pubKeyBytes, message, signature)
 ```
 
-Use this when your signer uses `ed25519.Sign(privKey, rawBytes)` (default in every Solana SDK / nacl library). `message` may be any length, not just 32 bytes.
+Use this when your signer uses `ed25519.Sign(privKey, rawBytes)` (default in every Solana SDK / nacl library). `message` may be any length up to `MaxEd25519MessageBytes` (128 KiB), not just 32 bytes.
 
 ### Common rules
 
 - `pubKey` must be exactly 32 bytes; `signature` must be exactly 64 bytes — otherwise the precompile reverts with `invalid params`.
+- `verifyEd25519RawMessage` reverts with `message too large` past `MaxEd25519MessageBytes` (128 KiB).
 - Unknown method IDs revert with the standard `unknown method` error.
-- Both methods cost `4000` gas.
+- `verifyEd25519` costs a flat `4000` gas; `verifyEd25519RawMessage` costs `4000` plus `12` per 32-byte word of `message`.
 
 ## Generating the ABI
 
@@ -120,7 +146,8 @@ If the call returns `0x` (empty), the precompile is not in `active_static_precom
 precompiles/usigverifier/
 |-- USigVerifier.sol     Solidity interface (the source of truth for the ABI)
 |-- abi.json             Embedded into the binary via go:embed
-|-- usigverifier.go      Precompile struct, NewPrecompile / NewPrecompileV2, RequiredGas, Run
-|-- query.go             VerifyEd25519 method handler
+|-- usigverifier.go      Precompile struct, NewPrecompile / NewPrecompileV2, RequiredGas (gas schedule), Run
+|-- query.go             VerifyEd25519 / VerifyEd25519RawMessage method handlers
+|-- gas_test.go          Gas-schedule + size-cap regression tests and benchmarks
 +-- README.md            (this file)
 ```