From d2a259093763e96fb455b788e522b98e49ccba07 Mon Sep 17 00:00:00 2001 From: Nilesh Gupta Date: Wed, 26 Aug 2026 06:51:20 +0530 Subject: [PATCH] fix: skip quadratic base58 decode outside the 64-byte signature length band Only 64..88 base58 chars can decode to 64 bytes, so gating the decode on that band is output-equivalent. Also cap tx_hash on the unauthenticated InboundKeys query. --- utils/canonical.go | 21 +++++- utils/canonical_test.go | 92 ++++++++++++++++++++++++--- x/uexecutor/keeper/query_keys.go | 16 +++++ x/uexecutor/keeper/query_keys_test.go | 87 +++++++++++++++++++++++++ 4 files changed, 205 insertions(+), 11 deletions(-) create mode 100644 x/uexecutor/keeper/query_keys_test.go diff --git a/utils/canonical.go b/utils/canonical.go index 64c5bc62..876a3c27 100644 --- a/utils/canonical.go +++ b/utils/canonical.go @@ -20,6 +20,18 @@ const ( const base58Alphabet = "123456789ABCDEFGHJKLMNPQRSTUVWXYZabcdefghijkmnopqrstuvwxyz" +// A base58-encoded 64-byte Solana signature is always 64..88 characters: 88 is +// ceil(512 / log2(58)) for a full-range value, and 64 is the all-zero case +// (each leading zero byte encodes as one '1'). Outside that band the decode can +// never produce 64 bytes, so its result would be discarded — see +// canonicalizeSolanaTxHash. mr-tron/base58's decoder is quadratic (for each of +// n characters it walks ceil(n/4) limbs), so decoding attacker-supplied strings +// only to throw the result away is an unmetered CPU sink on public query paths. +const ( + solanaSigBase58MinLen = 64 + solanaSigBase58MaxLen = 88 +) + // CAIP2Namespace returns the namespace component of a CAIP-2 chain id // ("eip155:1" → "eip155"). Returns "" when the id has no namespace. func CAIP2Namespace(chain string) string { @@ -119,8 +131,13 @@ func canonicalizeSolanaTxHash(s string) (string, error) { if strings.HasPrefix(canon, "0x") { return canon, nil } - if raw, decErr := base58.Decode(canon); decErr == nil && len(raw) == 64 { - return "0x" + hex.EncodeToString(raw), nil + // Only attempt the decode for lengths that can actually yield 64 bytes. + // This is output-equivalent for every possible input: a string outside the + // band already falls through to `return canon` below, decode or not. + if n := len(canon); n >= solanaSigBase58MinLen && n <= solanaSigBase58MaxLen { + if raw, decErr := base58.Decode(canon); decErr == nil && len(raw) == 64 { + return "0x" + hex.EncodeToString(raw), nil + } } return canon, nil } diff --git a/utils/canonical_test.go b/utils/canonical_test.go index 15ba309e..c7c1af0d 100644 --- a/utils/canonical_test.go +++ b/utils/canonical_test.go @@ -1,23 +1,28 @@ package utils_test import ( + "encoding/hex" + "math/rand" + "strings" "testing" + "time" + "github.com/mr-tron/base58" "github.com/stretchr/testify/require" "github.com/pushchain/push-chain-node/utils" ) const ( - eip55Addr = "0x5aAeb6053F3E94C9b9A09f33669435E7Ef1BeAed" - lowerAddr = "0x5aaeb6053f3e94c9b9a09f33669435e7ef1beaed" - upperAddr = "0X5AAEB6053F3E94C9B9A09F33669435E7EF1BEAED" - noPfxAddr = "5aaeb6053f3e94c9b9a09f33669435e7ef1beaed" - mixedHash = "0xB28F49668e7e76dc96D7aaBE5b7f63FEcfbd1c3574774c05e8204e749fd96fbd" - lowerHash = "0xb28f49668e7e76dc96d7aabe5b7f63fecfbd1c3574774c05e8204e749fd96fbd" - noPfxHash = "b28f49668e7e76dc96d7aabe5b7f63fecfbd1c3574774c05e8204e749fd96fbd" - solPubkey = "EPjFWdd5AufqSSqeM2qN1xzybapC8G4wEGGkZwyTDt1v" - solSig = "5j7s6NiJS3JAkvgkoc18WVAsiSaci2pxB2A6ueCJP4tprA2TFg9wSyTLeYouxPBJEMzJinENTkpA52YStRW5Dia7" + eip55Addr = "0x5aAeb6053F3E94C9b9A09f33669435E7Ef1BeAed" + lowerAddr = "0x5aaeb6053f3e94c9b9a09f33669435e7ef1beaed" + upperAddr = "0X5AAEB6053F3E94C9B9A09F33669435E7EF1BEAED" + noPfxAddr = "5aaeb6053f3e94c9b9a09f33669435e7ef1beaed" + mixedHash = "0xB28F49668e7e76dc96D7aaBE5b7f63FEcfbd1c3574774c05e8204e749fd96fbd" + lowerHash = "0xb28f49668e7e76dc96d7aabe5b7f63fecfbd1c3574774c05e8204e749fd96fbd" + noPfxHash = "b28f49668e7e76dc96d7aabe5b7f63fecfbd1c3574774c05e8204e749fd96fbd" + solPubkey = "EPjFWdd5AufqSSqeM2qN1xzybapC8G4wEGGkZwyTDt1v" + solSig = "5j7s6NiJS3JAkvgkoc18WVAsiSaci2pxB2A6ueCJP4tprA2TFg9wSyTLeYouxPBJEMzJinENTkpA52YStRW5Dia7" ) func TestCanonicalizeEVMAddress_EquivalentEncodingsConverge(t *testing.T) { @@ -134,3 +139,72 @@ func TestCAIP2Namespace(t *testing.T) { require.Equal(t, "solana", utils.CAIP2Namespace("solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1")) require.Equal(t, "", utils.CAIP2Namespace("no-colon")) } + +// referenceSolanaTxHash reproduces the pre-fix behaviour for pure-base58 input: +// decode unconditionally, convert only on an exact 64-byte result, otherwise +// return the input untouched. The length band added in canonicalizeSolanaTxHash +// must not change the result for any input. +func referenceSolanaTxHash(s string) string { + if raw, err := base58.Decode(s); err == nil && len(raw) == 64 { + return "0x" + hex.EncodeToString(raw) + } + return s +} + +func TestCanonicalizeTxHashByNamespace_Solana_LengthBandIsOutputEquivalent(t *testing.T) { + // Only 64..88 base58 chars can decode to exactly 64 bytes, so the band gate + // is a pure performance change. Sweep across it — 63/64/88/89 are the edges. + rng := rand.New(rand.NewSource(1)) + alphabet := []byte("123456789ABCDEFGHJKLMNPQRSTUVWXYZabcdefghijkmnopqrstuvwxyz") + + lengths := []int{1, 2, 31, 32, 43, 44, 63, 64, 65, 87, 88, 89, 90, 128, 200, 300} + for n := 3; n < 63; n += 7 { + lengths = append(lengths, n) + } + + for _, n := range lengths { + for variant := 0; variant < 4; variant++ { + b := make([]byte, n) + for i := range b { + switch variant { + case 0: + b[i] = '1' // all-zero decode: the short edge of the band + case 1: + b[i] = 'z' // largest digit: the long edge + default: + b[i] = alphabet[rng.Intn(len(alphabet))] + } + } + in := string(b) + require.Equal(t, referenceSolanaTxHash(in), + utils.LenientCanonicalizeTxHash("solana:devnet", in), + "length band changed the result for a %d-char input %q", n, in) + } + } +} + +func TestCanonicalizeTxHashByNamespace_Solana_RealSignatureStillConverges(t *testing.T) { + // The band must not break the case it exists to serve: an 88-char base58 + // signature still folds to 0x-hex. + got, err := utils.CanonicalizeTxHashByNamespace("solana:devnet", solSig) + require.NoError(t, err) + require.Equal(t, "0x", got[:2]) + require.Len(t, got, 2+128) +} + +func TestCanonicalizeTxHashByNamespace_Solana_OversizedInputDoesNotDecode(t *testing.T) { + // F-2026-18821: mr-tron/base58 decoding is quadratic, and the result for an + // out-of-band length is discarded. Before the fix a single 1e5-char decode + // measured 4.5-29s (and InboundKeys does three of them); after, no decode + // runs at all. The bound is loose enough not to flake on a busy CI box while + // still failing hard on any return to O(n^2). + huge := strings.Repeat("z", 100_000) + + start := time.Now() + got := utils.LenientCanonicalizeTxHash("solana:devnet", huge) + elapsed := time.Since(start) + + require.Equal(t, huge, got, "out-of-band input must pass through unchanged") + require.Less(t, elapsed, time.Second, + "oversized base58 tx_hash must not be decoded (took %s)", elapsed) +} diff --git a/x/uexecutor/keeper/query_keys.go b/x/uexecutor/keeper/query_keys.go index 503c2267..0708baad 100644 --- a/x/uexecutor/keeper/query_keys.go +++ b/x/uexecutor/keeper/query_keys.go @@ -12,6 +12,10 @@ import ( "github.com/pushchain/push-chain-node/x/uexecutor/types" ) +// maxQueryTxHashLen bounds the tx_hash accepted by the unauthenticated key +// derivation queries. Longest real value is an 88-char base58 Solana signature. +const maxQueryTxHashLen = 128 + // InboundKeys derives the canonical UTX id and inbound ballot id for the given // inbound, applying the same canonicalization the vote path uses. Lets off-chain // validators read the keys from the chain instead of re-implementing the rules. @@ -19,6 +23,18 @@ func (k Querier) InboundKeys(goCtx context.Context, req *types.QueryInboundKeysR if req == nil || req.Inbound == nil { return nil, status.Error(codes.InvalidArgument, "inbound is required") } + // This endpoint is unauthenticated, reads no state and so consumes no gas. + // Bound the one field that drives a decode (tx_hash) rather than trusting + // the caller. The limit is far above any real hash — 88 chars for a base58 + // Solana signature, 66 for 0x-prefixed EVM — so it rejects only garbage. + // Deliberately not applied to raw_payload / verification_data, which are + // legitimately long, nor pushed down into utils.Canonicalize*: the vote + // path must stay lenient (a malformed inbound still has to produce a UTX), + // and changing shared canonicalization would alter ballot keys. + if n := len(req.Inbound.TxHash); n > maxQueryTxHashLen { + return nil, status.Errorf(codes.InvalidArgument, + "tx_hash too long: %d chars (max %d)", n, maxQueryTxHashLen) + } inbound := *req.Inbound inbound.Canonicalize() diff --git a/x/uexecutor/keeper/query_keys_test.go b/x/uexecutor/keeper/query_keys_test.go new file mode 100644 index 00000000..51bbf64c --- /dev/null +++ b/x/uexecutor/keeper/query_keys_test.go @@ -0,0 +1,87 @@ +package keeper_test + +import ( + "strings" + "testing" + "time" + + "github.com/stretchr/testify/require" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + + "github.com/pushchain/push-chain-node/x/uexecutor/types" +) + +// solanaSig is a real 88-char base58 Solana signature (64 bytes). +const solanaSig = "5j7s6NiJS3JAkvgkoc18WVAsiSaci2pxB2A6ueCJP4tprA2TFg9wSyTLeYouxPBJEMzJinENTkpA52YStRW5Dia7" + +func TestInboundKeys_RejectsOversizedTxHash(t *testing.T) { + // F-2026-18821: InboundKeys is unauthenticated, reads no state and so burns + // no gas. It canonicalizes tx_hash three times (Canonicalize, then the UTX + // and ballot key helpers), and base58 decoding is quadratic — a 1e5-char + // hash cost tens of seconds of CPU per request before the fix. + f := SetupTest(t) + + huge := strings.Repeat("z", 100_000) + + start := time.Now() + _, err := f.queryServer.InboundKeys(f.ctx, &types.QueryInboundKeysRequest{ + Inbound: &types.Inbound{ + SourceChain: "solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1", + TxHash: huge, + LogIndex: "0", + TxType: types.TxType_FUNDS, + }, + }) + elapsed := time.Since(start) + + require.Error(t, err) + require.Equal(t, codes.InvalidArgument, status.Code(err)) + require.Contains(t, err.Error(), "tx_hash too long") + require.Less(t, elapsed, time.Second, "oversized tx_hash must fail fast (took %s)", elapsed) +} + +func TestInboundKeys_AcceptsRealSolanaSignature(t *testing.T) { + // The cap must not reject anything real: 88 chars is the longest a base58 + // 64-byte signature can be. + f := SetupTest(t) + + resp, err := f.queryServer.InboundKeys(f.ctx, &types.QueryInboundKeysRequest{ + Inbound: &types.Inbound{ + SourceChain: "solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1", + TxHash: solanaSig, + LogIndex: "0", + TxType: types.TxType_FUNDS, + }, + }) + + require.NoError(t, err) + require.NotEmpty(t, resp.UtxId) + require.NotEmpty(t, resp.BallotId) + // Canonicalization folds the base58 signature into 0x-hex. + require.Equal(t, "0x", resp.CanonicalInbound.TxHash[:2]) + require.Len(t, resp.CanonicalInbound.TxHash, 2+128) +} + +func TestInboundKeys_TxHashAtCapIsAccepted(t *testing.T) { + // Boundary: exactly maxQueryTxHashLen (128) is allowed, 129 is not. + f := SetupTest(t) + + newReq := func(n int) *types.QueryInboundKeysRequest { + return &types.QueryInboundKeysRequest{ + Inbound: &types.Inbound{ + SourceChain: "solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1", + TxHash: strings.Repeat("z", n), + LogIndex: "0", + TxType: types.TxType_FUNDS, + }, + } + } + + _, err := f.queryServer.InboundKeys(f.ctx, newReq(128)) + require.NoError(t, err, "128-char tx_hash is at the cap and must be accepted") + + _, err = f.queryServer.InboundKeys(f.ctx, newReq(129)) + require.Error(t, err) + require.Equal(t, codes.InvalidArgument, status.Code(err)) +}