Skip to content

Commit 3e0d76b

Browse files
committed
Merge remote-tracking branch 'origin/audit-fixes' into F-2026-18139
# Conflicts: # universalClient/chains/evm/event_confirmer.go
2 parents 9e86fed + 49f8e7c commit 3e0d76b

78 files changed

Lines changed: 7276 additions & 474 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

app/ante/account_init_decorator.go

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package ante
22

33
import (
4+
"bytes"
45
"fmt"
56

67
sdk "github.com/cosmos/cosmos-sdk/types"
@@ -12,6 +13,7 @@ import (
1213
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
1314
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
1415
"github.com/cosmos/cosmos-sdk/types/tx/signing"
16+
"github.com/cosmos/cosmos-sdk/x/auth/ante"
1517
authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing"
1618
txpolicy "github.com/pushchain/push-chain-node/app/txpolicy"
1719
)
@@ -55,7 +57,7 @@ func (aid AccountInitDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate
5557
"address", sdk.AccAddress(newAccAddr).String(),
5658
"simulate", simulate,
5759
)
58-
// if account does not exist on chain, bypass rest of ante chain (especially gas and signature verification) here.
60+
// if account does not exist on chain, bypass rest of ante chain here.
5961
// Perform signature verification on account number e and sequence number e instead.
6062
if err := aid.verifySignatureForNewAccount(ctx, tx, simulate); err != nil {
6163
ctx.Logger().Debug("account init decorator: signature verification failed for new account",
@@ -103,13 +105,52 @@ func (aid AccountInitDecorator) verifySignatureForNewAccount(ctx sdk.Context, tx
103105
return errorsmod.Wrapf(sdkerrors.ErrUnauthorized, "invalid number of signer; expected: %d, got %d", len(signers), len(sigs))
104106
}
105107

106-
newAccAddr := sdk.AccAddress(signers[0])
108+
params := aid.ak.GetParams(ctx)
109+
110+
// Enforce the signature count limit before doing any verification work.
111+
// This decorator short-circuits the ante chain for new accounts, so
112+
// ante.ValidateSigCountDecorator never runs for them; without this hard cap
113+
// a gasless tx could carry an arbitrarily large multisig key and force the
114+
// node to verify every sub-signature. Gas is deliberately NOT consumed here:
115+
// gasless txs skip fee deduction entirely, so charging gas would cost an
116+
// attacker nothing - the count cap is what actually bounds the work.
117+
sigCount := 0
107118
for _, sig := range sigs {
119+
if sig.PubKey == nil {
120+
return errorsmod.Wrap(sdkerrors.ErrInvalidPubKey, "pubkey is not provided in signature")
121+
}
122+
sigCount += ante.CountSubKeys(sig.PubKey)
123+
if uint64(sigCount) > params.TxSigLimit {
124+
return errorsmod.Wrapf(sdkerrors.ErrTooManySignatures,
125+
"signatures: %d, limit: %d", sigCount, params.TxSigLimit)
126+
}
127+
}
128+
129+
newAccAddr := sdk.AccAddress(signers[0])
130+
for i, sig := range sigs {
108131
pubKey := sig.PubKey
109132
if pubKey == nil {
110133
return errorsmod.Wrap(sdkerrors.ErrInvalidPubKey, "pubkey is not provided in signature")
111134
}
112135

136+
// Bind the declared signer to the key that actually signed the tx.
137+
//
138+
// VerifySignature below only proves "this key signed this tx"; it says
139+
// nothing about WHO the tx claims to be from. Because this decorator
140+
// short-circuits the ante chain for new accounts, the SDK's
141+
// SetPubKeyDecorator - which owns this check - never runs, so a tx could
142+
// declare an arbitrary signer while being signed by an unrelated key.
143+
// Bech32 account addresses may be up to 255 bytes, and downstream
144+
// conversion to a 20-byte EVM address keeps only the rightmost bytes, so
145+
// a crafted longer signer could alias a module address.
146+
//
147+
// Guards mirror x/auth/ante/sigverify.go exactly so simulation and gas
148+
// estimation keep working.
149+
if !simulate && ctx.IsSigverifyTx() && !bytes.Equal(pubKey.Address().Bytes(), signers[i]) {
150+
return errorsmod.Wrapf(sdkerrors.ErrInvalidPubKey,
151+
"pubKey does not match signer address %s with signer index: %d", sdk.AccAddress(signers[i]).String(), i)
152+
}
153+
113154
// retrieve signer data
114155
chainID := ctx.ChainID()
115156
var accSequence uint64 = 0
Lines changed: 298 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,298 @@
1+
package ante_test
2+
3+
import (
4+
"context"
5+
"fmt"
6+
"testing"
7+
8+
kmultisig "github.com/cosmos/cosmos-sdk/crypto/keys/multisig"
9+
"github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1"
10+
cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
11+
sdk "github.com/cosmos/cosmos-sdk/types"
12+
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
13+
"github.com/ethereum/go-ethereum/common"
14+
"github.com/stretchr/testify/require"
15+
16+
clienttx "github.com/cosmos/cosmos-sdk/client/tx"
17+
"github.com/cosmos/cosmos-sdk/std"
18+
"github.com/cosmos/cosmos-sdk/types/tx/signing"
19+
authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing"
20+
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
21+
22+
"github.com/pushchain/push-chain-node/app/ante"
23+
appparams "github.com/pushchain/push-chain-node/app/params"
24+
uexecutortypes "github.com/pushchain/push-chain-node/x/uexecutor/types"
25+
)
26+
27+
// uexecutorModuleEVMAddr is the EVM address of the uexecutor module account -
28+
// sha256("uexecutor")[:20]. The UEA contract trusts calls coming from it
29+
// unconditionally, which is what makes aliasing onto it so damaging.
30+
const uexecutorModuleEVMAddr = "0x14191Ea54B4c176fCf86f51b0FAc7CB1E71Df7d7"
31+
32+
const anteTestChainID = "push_9000-1"
33+
34+
// newSignerBindingEncodingConfig returns an encoding config able to build and
35+
// sign real uexecutor transactions.
36+
func newSignerBindingEncodingConfig(t *testing.T) appparams.EncodingConfig {
37+
t.Helper()
38+
encCfg := appparams.MakeEncodingConfig()
39+
std.RegisterInterfaces(encCfg.InterfaceRegistry)
40+
authtypes.RegisterInterfaces(encCfg.InterfaceRegistry)
41+
uexecutortypes.RegisterInterfaces(encCfg.InterfaceRegistry)
42+
return encCfg
43+
}
44+
45+
// aliasedSigner returns a `length`-byte address whose RIGHTMOST 20 bytes are the
46+
// uexecutor module account. common.BytesToAddress keeps exactly those bytes, so
47+
// every such address collapses onto the module's EVM address.
48+
func aliasedSigner(t *testing.T, length int) sdk.AccAddress {
49+
t.Helper()
50+
require.Greater(t, length, common.AddressLength)
51+
52+
moduleAddr := authtypes.NewModuleAddress(uexecutortypes.ModuleName)
53+
require.Len(t, moduleAddr, common.AddressLength)
54+
require.Equal(t, uexecutorModuleEVMAddr, common.BytesToAddress(moduleAddr).Hex())
55+
56+
prefix := make([]byte, length-common.AddressLength)
57+
prefix[0] = 0x01
58+
addr := sdk.AccAddress(append(prefix, moduleAddr...))
59+
require.Len(t, addr, length)
60+
61+
// The whole point of the finding: this longer address truncates onto the
62+
// module's EVM address downstream.
63+
require.Equal(t, uexecutorModuleEVMAddr, common.BytesToAddress(addr).Hex())
64+
return addr
65+
}
66+
67+
// gaslessMsgFor builds one of the two user-facing gasless messages with the
68+
// given declared signer.
69+
func gaslessMsgFor(t *testing.T, msgType string, signer sdk.AccAddress) sdk.Msg {
70+
t.Helper()
71+
ua := &uexecutortypes.UniversalAccountId{
72+
ChainNamespace: "eip155",
73+
ChainId: "11155111",
74+
Owner: "0x000000000000000000000000000000000000dead",
75+
}
76+
77+
switch msgType {
78+
case "MsgExecutePayload":
79+
return &uexecutortypes.MsgExecutePayload{
80+
Signer: signer.String(),
81+
UniversalAccountId: ua,
82+
UniversalPayload: &uexecutortypes.UniversalPayload{
83+
To: "0x000000000000000000000000000000000000dead",
84+
Data: "0xabcdef",
85+
},
86+
VerificationData: "0xabcdef",
87+
}
88+
case "MsgMigrateUEA":
89+
return &uexecutortypes.MsgMigrateUEA{
90+
Signer: signer.String(),
91+
UniversalAccountId: ua,
92+
MigrationPayload: &uexecutortypes.MigrationPayload{
93+
Migration: "0x000000000000000000000000000000000000beef",
94+
Nonce: "0",
95+
Deadline: "1",
96+
},
97+
Signature: "0xabcdef",
98+
}
99+
default:
100+
t.Fatalf("unknown msg type %q", msgType)
101+
return nil
102+
}
103+
}
104+
105+
// buildSignedTx returns a tx carrying msg whose declared signer is
106+
// `declaredSigner` but which is signed by `priv` - the two need not be related,
107+
// which is exactly the confusion the fix has to reject.
108+
func buildSignedTx(t *testing.T, encCfg appparams.EncodingConfig, msg sdk.Msg, declaredSigner sdk.AccAddress, priv cryptotypes.PrivKey) sdk.Tx {
109+
t.Helper()
110+
111+
txb := encCfg.TxConfig.NewTxBuilder()
112+
require.NoError(t, txb.SetMsgs(msg))
113+
txb.SetGasLimit(300_000)
114+
115+
require.NoError(t, txb.SetSignatures(signing.SignatureV2{
116+
PubKey: priv.PubKey(),
117+
Data: &signing.SingleSignatureData{SignMode: signing.SignMode_SIGN_MODE_DIRECT},
118+
Sequence: 0,
119+
}))
120+
121+
// The gasless new-account path signs over account number 0 / sequence 0,
122+
// since the account does not exist on chain yet.
123+
signerData := authsigning.SignerData{
124+
Address: declaredSigner.String(),
125+
ChainID: anteTestChainID,
126+
AccountNumber: 0,
127+
Sequence: 0,
128+
PubKey: priv.PubKey(),
129+
}
130+
131+
sigV2, err := clienttx.SignWithPrivKey(
132+
context.Background(), signing.SignMode_SIGN_MODE_DIRECT, signerData,
133+
txb, priv, encCfg.TxConfig, 0,
134+
)
135+
require.NoError(t, err)
136+
require.NoError(t, txb.SetSignatures(sigV2))
137+
138+
return txb.GetTx()
139+
}
140+
141+
func newSignerBindingDecorator(t *testing.T, encCfg appparams.EncodingConfig) (ante.AccountInitDecorator, *mockAccountKeeperAnte) {
142+
t.Helper()
143+
ak := newMockAccountKeeperAnte(sdk.AccAddress([]byte("feeCollector")))
144+
return ante.NewAccountInitDecorator(ak, encCfg.TxConfig.SignModeHandler()), ak
145+
}
146+
147+
// TestAccountInitDecorator_RejectsAliasedModuleSigner is the regression test for
148+
// F-2026-18200: a gasless tx may not declare an over-long signer that truncates
149+
// onto the uexecutor module address while being signed by an unrelated key.
150+
//
151+
// Hacken's PoC only used the 21-byte case; truncation works for ANY length > 20,
152+
// so 21, 22 and 32 bytes are all covered, against both gasless messages.
153+
func TestAccountInitDecorator_RejectsAliasedModuleSigner(t *testing.T) {
154+
encCfg := newSignerBindingEncodingConfig(t)
155+
156+
for _, msgType := range []string{"MsgExecutePayload", "MsgMigrateUEA"} {
157+
for _, length := range []int{21, 22, 32} {
158+
t.Run(fmt.Sprintf("%s/%dbytes", msgType, length), func(t *testing.T) {
159+
attackerKey := secp256k1.GenPrivKey()
160+
declaredSigner := aliasedSigner(t, length)
161+
msg := gaslessMsgFor(t, msgType, declaredSigner)
162+
tx := buildSignedTx(t, encCfg, msg, declaredSigner, attackerKey)
163+
164+
aid, ak := newSignerBindingDecorator(t, encCfg)
165+
ctx := newAnteTestCtx(t, false).WithChainID(anteTestChainID)
166+
167+
nextCalled := false
168+
_, err := aid.AnteHandle(ctx, tx, false, func(ctx sdk.Context, tx sdk.Tx, simulate bool) (sdk.Context, error) {
169+
nextCalled = true
170+
return ctx, nil
171+
})
172+
173+
require.Error(t, err, "aliased signer must not pass the ante chain")
174+
require.True(t, sdkerrors.ErrInvalidPubKey.Is(err), "expected ErrInvalidPubKey, got: %v", err)
175+
require.False(t, nextCalled, "the message must never reach execution")
176+
require.False(t, ak.HasAccount(context.Background(), declaredSigner),
177+
"no account may be persisted for a rejected signer")
178+
})
179+
}
180+
}
181+
}
182+
183+
// TestAccountInitDecorator_RejectsMismatchedSigner covers the general case: a
184+
// well-formed 20-byte signer that is simply not the address of the signing key.
185+
func TestAccountInitDecorator_RejectsMismatchedSigner(t *testing.T) {
186+
encCfg := newSignerBindingEncodingConfig(t)
187+
188+
attackerKey := secp256k1.GenPrivKey()
189+
victimKey := secp256k1.GenPrivKey()
190+
declaredSigner := sdk.AccAddress(victimKey.PubKey().Address())
191+
192+
msg := gaslessMsgFor(t, "MsgExecutePayload", declaredSigner)
193+
tx := buildSignedTx(t, encCfg, msg, declaredSigner, attackerKey)
194+
195+
aid, ak := newSignerBindingDecorator(t, encCfg)
196+
ctx := newAnteTestCtx(t, false).WithChainID(anteTestChainID)
197+
198+
_, err := aid.AnteHandle(ctx, tx, false, emptyNext)
199+
require.Error(t, err)
200+
require.True(t, sdkerrors.ErrInvalidPubKey.Is(err), "expected ErrInvalidPubKey, got: %v", err)
201+
require.False(t, ak.HasAccount(context.Background(), declaredSigner))
202+
}
203+
204+
// TestAccountInitDecorator_AcceptsMatchingSigner is the positive control: a
205+
// normal 20-byte signer whose key matches still creates the account and passes.
206+
func TestAccountInitDecorator_AcceptsMatchingSigner(t *testing.T) {
207+
encCfg := newSignerBindingEncodingConfig(t)
208+
209+
for _, msgType := range []string{"MsgExecutePayload", "MsgMigrateUEA"} {
210+
t.Run(msgType, func(t *testing.T) {
211+
key := secp256k1.GenPrivKey()
212+
signer := sdk.AccAddress(key.PubKey().Address())
213+
require.Len(t, signer, common.AddressLength)
214+
215+
msg := gaslessMsgFor(t, msgType, signer)
216+
tx := buildSignedTx(t, encCfg, msg, signer, key)
217+
218+
aid, ak := newSignerBindingDecorator(t, encCfg)
219+
ctx := newAnteTestCtx(t, false).WithChainID(anteTestChainID)
220+
221+
_, err := aid.AnteHandle(ctx, tx, false, emptyNext)
222+
require.NoError(t, err)
223+
224+
acc := ak.GetAccount(context.Background(), signer)
225+
require.NotNil(t, acc, "the account must be created for a legitimate gasless tx")
226+
require.Equal(t, uint64(1), acc.GetSequence())
227+
})
228+
}
229+
}
230+
231+
// TestAccountInitDecorator_SimulationUnaffected checks that the new binding
232+
// check keeps the SDK's `!simulate` guard, so simulation and gas estimation -
233+
// which carry no usable signature - keep working.
234+
func TestAccountInitDecorator_SimulationUnaffected(t *testing.T) {
235+
encCfg := newSignerBindingEncodingConfig(t)
236+
237+
attackerKey := secp256k1.GenPrivKey()
238+
victimKey := secp256k1.GenPrivKey()
239+
240+
for name, declaredSigner := range map[string]sdk.AccAddress{
241+
"matching_signer": sdk.AccAddress(attackerKey.PubKey().Address()),
242+
"mismatched_signer": sdk.AccAddress(victimKey.PubKey().Address()),
243+
} {
244+
t.Run(name, func(t *testing.T) {
245+
msg := gaslessMsgFor(t, "MsgExecutePayload", declaredSigner)
246+
tx := buildSignedTx(t, encCfg, msg, declaredSigner, attackerKey)
247+
248+
aid, _ := newSignerBindingDecorator(t, encCfg)
249+
ctx := newAnteTestCtx(t, false).WithChainID(anteTestChainID)
250+
251+
_, err := aid.AnteHandle(ctx, tx, true /* simulate */, emptyNext)
252+
require.NoError(t, err, "simulation must not be affected by the binding check")
253+
})
254+
}
255+
}
256+
257+
// TestAccountInitDecorator_EnforcesSignatureLimit covers F-2026-18186: the
258+
// new-account path short-circuits the ante chain, so it has to enforce the
259+
// signature count limit itself instead of verifying an unbounded multisig for
260+
// free.
261+
func TestAccountInitDecorator_EnforcesSignatureLimit(t *testing.T) {
262+
encCfg := newSignerBindingEncodingConfig(t)
263+
264+
params := authtypes.DefaultParams()
265+
numKeys := int(params.TxSigLimit) + 1
266+
267+
pubKeys := make([]cryptotypes.PubKey, numKeys)
268+
sigs := make([]signing.SignatureData, numKeys)
269+
bitArray := cryptotypes.NewCompactBitArray(numKeys)
270+
for i := 0; i < numKeys; i++ {
271+
pubKeys[i] = secp256k1.GenPrivKey().PubKey()
272+
sigs[i] = &signing.SingleSignatureData{
273+
SignMode: signing.SignMode_SIGN_MODE_DIRECT,
274+
Signature: []byte("not-checked-the-limit-trips-first"),
275+
}
276+
bitArray.SetIndex(i, true)
277+
}
278+
279+
multisigPk := kmultisig.NewLegacyAminoPubKey(numKeys, pubKeys)
280+
signer := sdk.AccAddress(multisigPk.Address())
281+
282+
txb := encCfg.TxConfig.NewTxBuilder()
283+
require.NoError(t, txb.SetMsgs(gaslessMsgFor(t, "MsgExecutePayload", signer)))
284+
txb.SetGasLimit(300_000)
285+
require.NoError(t, txb.SetSignatures(signing.SignatureV2{
286+
PubKey: multisigPk,
287+
Data: &signing.MultiSignatureData{BitArray: bitArray, Signatures: sigs},
288+
Sequence: 0,
289+
}))
290+
291+
aid, ak := newSignerBindingDecorator(t, encCfg)
292+
ctx := newAnteTestCtx(t, false).WithChainID(anteTestChainID)
293+
294+
_, err := aid.AnteHandle(ctx, txb.GetTx(), false, emptyNext)
295+
require.Error(t, err)
296+
require.True(t, sdkerrors.ErrTooManySignatures.Is(err), "expected ErrTooManySignatures, got: %v", err)
297+
require.False(t, ak.HasAccount(context.Background(), signer))
298+
}

0 commit comments

Comments
 (0)