diff --git a/app/ante/evm_checktx.go b/app/ante/evm_checktx.go index 73878163ce..5e9cc0d7fe 100644 --- a/app/ante/evm_checktx.go +++ b/app/ante/evm_checktx.go @@ -75,45 +75,11 @@ func EvmCheckTxAnte( } func EvmStatelessChecks(ctx sdk.Context, tx sdk.Tx, chainID *big.Int) error { - txBody, ok := tx.(TxBody) - if ok { - body := txBody.GetBody() - if body.Memo != "" { - return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "memo must be empty for EVM txs") - } - if body.TimeoutHeight != 0 { - return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "timeout_height must be zero for EVM txs") - } - if len(body.ExtensionOptions) > 0 || len(body.NonCriticalExtensionOptions) > 0 { - return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "extension options must be empty for EVM txs") - } - } - - txAuth, ok := tx.(TxAuthInfo) - if ok { - authInfo := txAuth.GetAuthInfo() - if len(authInfo.SignerInfos) > 0 { - return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "signer_infos must be empty for EVM txs") - } - if authInfo.Fee != nil { - if len(authInfo.Fee.Amount) > 0 { - return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "fee amount must be empty for EVM txs") - } - if authInfo.Fee.Payer != "" || authInfo.Fee.Granter != "" { - return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "fee payer and granter must be empty for EVM txs") - } - } + if err := evmante.ValidateNoCosmosTxFields(tx); err != nil { + return err } - - txSig, ok := tx.(TxSignaturesV2) - if ok { - sigs, err := txSig.GetSignaturesV2() - if err != nil { - return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "could not get signatures") - } - if len(sigs) > 0 { - return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "signatures must be empty for EVM txs") - } + if err := evmante.ValidateNoOuterEVMMsgFields(tx); err != nil { + return err } if len(tx.GetMsgs()) != 1 { diff --git a/app/ante/evm_checktx_test.go b/app/ante/evm_checktx_test.go new file mode 100644 index 0000000000..0942fc30d4 --- /dev/null +++ b/app/ante/evm_checktx_test.go @@ -0,0 +1,32 @@ +package ante + +import ( + "testing" + + "github.com/stretchr/testify/require" + + codectypes "github.com/sei-protocol/sei-chain/sei-cosmos/codec/types" + sdk "github.com/sei-protocol/sei-chain/sei-cosmos/types" + txtypes "github.com/sei-protocol/sei-chain/sei-cosmos/types/tx" + authtx "github.com/sei-protocol/sei-chain/sei-cosmos/x/auth/tx" + evmtypes "github.com/sei-protocol/sei-chain/x/evm/types" + "github.com/sei-protocol/sei-chain/x/evm/types/ethtx" +) + +func TestEvmStatelessChecksRejectsRawSignatures(t *testing.T) { + msg, err := evmtypes.NewMsgEVMTransaction(ðtx.AssociateTx{}) + require.NoError(t, err) + msgAny, err := codectypes.NewAnyWithValue(msg) + require.NoError(t, err) + + protoTx := &txtypes.Tx{ + Body: &txtypes.TxBody{Messages: []*codectypes.Any{msgAny}}, + AuthInfo: &txtypes.AuthInfo{ + Fee: &txtypes.Fee{}, + }, + Signatures: [][]byte{{0x1}}, + } + + err = EvmStatelessChecks(sdk.Context{}, authtx.WrapTx(protoTx).GetTx(), nil) + require.ErrorContains(t, err, "signatures must be empty") +} diff --git a/giga/deps/xevm/types/message_evm_transaction.go b/giga/deps/xevm/types/message_evm_transaction.go index 498478df50..d024051f43 100644 --- a/giga/deps/xevm/types/message_evm_transaction.go +++ b/giga/deps/xevm/types/message_evm_transaction.go @@ -42,6 +42,12 @@ func (msg *MsgEVMTransaction) GetSignBytes() []byte { } func (msg *MsgEVMTransaction) ValidateBasic() error { + if msg.Derived != nil && msg.Derived.PubKey == nil { + return sdkerrors.ErrInvalidPubKey + } + if err := validateCanonicalTxData(msg.Data); err != nil { + return err + } amsg, isAssociate := msg.GetAssociateTx() if isAssociate && len(amsg.CustomMessage) > MaxAssociateCustomMessageLength { return sdkerrors.Wrapf(sdkerrors.ErrTxTooLarge, "custom message can have at most 64 characters") @@ -49,6 +55,25 @@ func (msg *MsgEVMTransaction) ValidateBasic() error { return nil } +func validateCanonicalTxData(any *codectypes.Any) error { + txData, err := UnpackTxData(any) + if err != nil { + return err + } + msg, ok := txData.(proto.Message) + if !ok { + return sdkerrors.Wrap(sdkerrors.ErrInvalidType, "invalid EVM tx data") + } + canonicalBz, err := proto.Marshal(msg) + if err != nil { + return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "could not marshal EVM tx data") + } + if len(any.Value) != len(canonicalBz) { + return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "EVM tx data contains unsupported fields") + } + return nil +} + func (msg *MsgEVMTransaction) AsTransaction() (*ethtypes.Transaction, ethtx.TxData) { txData, err := UnpackTxData(msg.Data) if err != nil { diff --git a/giga/deps/xevm/types/message_evm_transaction_test.go b/giga/deps/xevm/types/message_evm_transaction_test.go index 43c69688c1..cbe7ebc763 100644 --- a/giga/deps/xevm/types/message_evm_transaction_test.go +++ b/giga/deps/xevm/types/message_evm_transaction_test.go @@ -10,9 +10,11 @@ import ( "github.com/sei-protocol/sei-chain/app" testkeeper "github.com/sei-protocol/sei-chain/giga/deps/testutil/keeper" sdk "github.com/sei-protocol/sei-chain/sei-cosmos/types" + sdkerrors "github.com/sei-protocol/sei-chain/sei-cosmos/types/errors" wasmtypes "github.com/sei-protocol/sei-chain/sei-wasmd/x/wasm/types" "github.com/ethereum/go-ethereum/common" + "github.com/sei-protocol/sei-chain/giga/deps/xevm/derived" "github.com/sei-protocol/sei-chain/giga/deps/xevm/types" "github.com/sei-protocol/sei-chain/giga/deps/xevm/types/ethtx" "github.com/stretchr/testify/require" @@ -101,3 +103,19 @@ func TestMustGetEVMTransactionMessageMultipleMsgs(t *testing.T) { types.MustGetEVMTransactionMessage(testTx) t.Errorf("Should not be able to convert a non evm emssage") } + +func TestValidateBasicRejectsBloatInTxData(t *testing.T) { + msg, err := types.NewMsgEVMTransaction(ðtx.AssociateTx{}) + require.NoError(t, err) + + msg.Data.Value = append(msg.Data.Value, 0x28, 0x01) + + err = msg.ValidateBasic() + require.ErrorContains(t, err, "EVM tx data contains unsupported fields") +} + +func TestValidateBasicRejectsExternalDerived(t *testing.T) { + msg := &types.MsgEVMTransaction{Derived: &derived.Derived{SenderEVMAddr: common.BytesToAddress([]byte("abc"))}} + err := msg.ValidateBasic() + require.ErrorIs(t, err, sdkerrors.ErrInvalidPubKey) +} diff --git a/x/evm/ante/field_bloat.go b/x/evm/ante/field_bloat.go new file mode 100644 index 0000000000..970cfecd3e --- /dev/null +++ b/x/evm/ante/field_bloat.go @@ -0,0 +1,126 @@ +package ante + +import ( + "github.com/gogo/protobuf/proto" + sdk "github.com/sei-protocol/sei-chain/sei-cosmos/types" + sdkerrors "github.com/sei-protocol/sei-chain/sei-cosmos/types/errors" + txtypes "github.com/sei-protocol/sei-chain/sei-cosmos/types/tx" + signing "github.com/sei-protocol/sei-chain/sei-cosmos/types/tx/signing" + evmtypes "github.com/sei-protocol/sei-chain/x/evm/types" +) + +type protoTx interface { + GetProtoTx() *txtypes.Tx +} + +// ValidateNoCosmosTxFields rejects Cosmos wrapper fields that EVM txs must not use. +func ValidateNoCosmosTxFields(tx sdk.Tx) error { + txBody, ok := tx.(interface { + GetBody() *txtypes.TxBody + }) + if ok { + body := txBody.GetBody() + if body.Memo != "" { + return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "memo must be empty for EVM txs") + } + if body.TimeoutHeight != 0 { + return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "timeout_height must be zero for EVM txs") + } + if len(body.ExtensionOptions) > 0 || len(body.NonCriticalExtensionOptions) > 0 { + return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "extension options must be empty for EVM txs") + } + } + + txAuth, ok := tx.(interface { + GetAuthInfo() *txtypes.AuthInfo + }) + if ok { + authInfo := txAuth.GetAuthInfo() + if len(authInfo.SignerInfos) > 0 { + return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "signer_infos must be empty for EVM txs") + } + if authInfo.Fee != nil { + if len(authInfo.Fee.Amount) > 0 { + return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "fee amount must be empty for EVM txs") + } + if authInfo.Fee.Payer != "" || authInfo.Fee.Granter != "" { + return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "fee payer and granter must be empty for EVM txs") + } + } + } + + if txProto, ok := tx.(protoTx); ok { + if len(txProto.GetProtoTx().Signatures) > 0 { + return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "signatures must be empty for EVM txs") + } + return nil + } + + txSig, ok := tx.(interface { + GetSignaturesV2() ([]signing.SignatureV2, error) + }) + if ok { + sigs, err := txSig.GetSignaturesV2() + if err != nil { + return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "could not get signatures") + } + if len(sigs) > 0 { + return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "signatures must be empty for EVM txs") + } + } + + return nil +} + +// ValidateNoOuterEVMMsgFields rejects extra bytes on the serialized outer MsgEVMTransaction. +func ValidateNoOuterEVMMsgFields(tx sdk.Tx) error { + txProto, ok := tx.(protoTx) + if !ok { + return nil + } + + protoTx := txProto.GetProtoTx() + if protoTx == nil || protoTx.Body == nil || len(protoTx.Body.Messages) != 1 || len(tx.GetMsgs()) != 1 { + return nil + } + + msg, ok := tx.GetMsgs()[0].(*evmtypes.MsgEVMTransaction) + if !ok || msg.Data == nil || protoTx.Body.Messages[0] == nil { + return nil + } + + canonicalMsgBz, err := canonicalEVMMsgBytes(msg) + if err != nil { + return err + } + if len(protoTx.Body.Messages[0].Value) != len(canonicalMsgBz) { + return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "EVM message contains unsupported fields") + } + return nil +} + +func canonicalEVMMsgBytes(msg *evmtypes.MsgEVMTransaction) ([]byte, error) { + txData, err := evmtypes.UnpackTxData(msg.Data) + if err != nil { + return nil, err + } + txDataMsg, ok := txData.(proto.Message) + if !ok { + return nil, sdkerrors.Wrap(sdkerrors.ErrInvalidType, "invalid EVM tx data") + } + canonicalDataBz, err := proto.Marshal(txDataMsg) + if err != nil { + return nil, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "could not marshal EVM tx data") + } + + normalizedMsg := *msg + normalizedData := *msg.Data + normalizedData.Value = canonicalDataBz + normalizedMsg.Data = &normalizedData + + canonicalMsgBz, err := normalizedMsg.Marshal() + if err != nil { + return nil, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "could not marshal EVM message") + } + return canonicalMsgBz, nil +} diff --git a/x/evm/ante/no_cosmos_fields.go b/x/evm/ante/no_cosmos_fields.go index 0a156db73d..a9c9dcc653 100644 --- a/x/evm/ante/no_cosmos_fields.go +++ b/x/evm/ante/no_cosmos_fields.go @@ -2,9 +2,6 @@ package ante import ( sdk "github.com/sei-protocol/sei-chain/sei-cosmos/types" - sdkerrors "github.com/sei-protocol/sei-chain/sei-cosmos/types/errors" - txtypes "github.com/sei-protocol/sei-chain/sei-cosmos/types/tx" - signing "github.com/sei-protocol/sei-chain/sei-cosmos/types/tx/signing" ) // EVMNoCosmosFieldsDecorator ensures all Cosmos tx fields are empty for EVM txs. @@ -15,51 +12,11 @@ func NewEVMNoCosmosFieldsDecorator() EVMNoCosmosFieldsDecorator { } func (d EVMNoCosmosFieldsDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) { - txBody, ok := tx.(interface { - GetBody() *txtypes.TxBody - }) - if ok { - body := txBody.GetBody() - if body.Memo != "" { - return ctx, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "memo must be empty for EVM txs") - } - if body.TimeoutHeight != 0 { - return ctx, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "timeout_height must be zero for EVM txs") - } - if len(body.ExtensionOptions) > 0 || len(body.NonCriticalExtensionOptions) > 0 { - return ctx, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "extension options must be empty for EVM txs") - } + if err := ValidateNoCosmosTxFields(tx); err != nil { + return ctx, err } - - txAuth, ok := tx.(interface { - GetAuthInfo() *txtypes.AuthInfo - }) - if ok { - authInfo := txAuth.GetAuthInfo() - if len(authInfo.SignerInfos) > 0 { - return ctx, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "signer_infos must be empty for EVM txs") - } - if authInfo.Fee != nil { - if len(authInfo.Fee.Amount) > 0 { - return ctx, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "fee amount must be empty for EVM txs") - } - if authInfo.Fee.Payer != "" || authInfo.Fee.Granter != "" { - return ctx, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "fee payer and granter must be empty for EVM txs") - } - } - } - - txSig, ok := tx.(interface { - GetSignaturesV2() ([]signing.SignatureV2, error) - }) - if ok { - sigs, err := txSig.GetSignaturesV2() - if err != nil { - return ctx, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "could not get signatures") - } - if len(sigs) > 0 { - return ctx, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "signatures must be empty for EVM txs") - } + if err := ValidateNoOuterEVMMsgFields(tx); err != nil { + return ctx, err } return next(ctx, tx, simulate) diff --git a/x/evm/ante/no_cosmos_fields_test.go b/x/evm/ante/no_cosmos_fields_test.go index 22b1baa1f9..8e7b0859c6 100644 --- a/x/evm/ante/no_cosmos_fields_test.go +++ b/x/evm/ante/no_cosmos_fields_test.go @@ -1,6 +1,7 @@ package ante_test import ( + "bytes" "testing" "github.com/stretchr/testify/require" @@ -8,8 +9,10 @@ import ( codectypes "github.com/sei-protocol/sei-chain/sei-cosmos/codec/types" sdk "github.com/sei-protocol/sei-chain/sei-cosmos/types" txtypes "github.com/sei-protocol/sei-chain/sei-cosmos/types/tx" - signing "github.com/sei-protocol/sei-chain/sei-cosmos/types/tx/signing" + authtx "github.com/sei-protocol/sei-chain/sei-cosmos/x/auth/tx" "github.com/sei-protocol/sei-chain/x/evm/ante" + evmtypes "github.com/sei-protocol/sei-chain/x/evm/types" + "github.com/sei-protocol/sei-chain/x/evm/types/ethtx" ) func TestEVMNoCosmosFieldsDecorator(t *testing.T) { @@ -18,9 +21,8 @@ func TestEVMNoCosmosFieldsDecorator(t *testing.T) { // Valid EVM tx: all forbidden fields empty tx := mockTx{ - body: &txtypes.TxBody{}, - authInfo: &txtypes.AuthInfo{Fee: &txtypes.Fee{}}, - signature: nil, + body: &txtypes.TxBody{}, + authInfo: &txtypes.AuthInfo{Fee: &txtypes.Fee{}}, } _, err := decorator.AnteHandle(ctx, tx, false, func(ctx sdk.Context, tx sdk.Tx, simulate bool) (sdk.Context, error) { return ctx, nil @@ -75,9 +77,57 @@ func TestEVMNoCosmosFieldsDecorator(t *testing.T) { require.ErrorContains(t, err, "fee payer and granter must be empty") tx.authInfo.Fee.Granter = "" - // Signatures set - tx.signature = []signing.SignatureV2{{}} - _, err = decorator.AnteHandle(ctx, tx, false, nil) +} + +func TestEVMNoCosmosFieldsDecoratorRejectsRawSignatures(t *testing.T) { + decorator := ante.NewEVMNoCosmosFieldsDecorator() + ctx := sdk.Context{} + tx := wrapRawProtoTx(t, mustNewMsgEVMTransaction(t), nil, [][]byte{{0x1}}) + + _, err := decorator.AnteHandle(ctx, tx, false, nil) require.ErrorContains(t, err, "signatures must be empty") - tx.signature = nil +} + +func TestEVMNoCosmosFieldsDecoratorRejectsOuterMessageBloat(t *testing.T) { + decorator := ante.NewEVMNoCosmosFieldsDecorator() + ctx := sdk.Context{} + + msg := mustNewMsgEVMTransaction(t) + msgAny, err := codectypes.NewAnyWithValue(msg) + require.NoError(t, err) + msgAny.Value = append(msgAny.Value, 0x12, 0x08) + msgAny.Value = append(msgAny.Value, bytes.Repeat([]byte{0xaa}, 8)...) + + protoTx := &txtypes.Tx{ + Body: &txtypes.TxBody{Messages: []*codectypes.Any{msgAny}}, + AuthInfo: &txtypes.AuthInfo{ + Fee: &txtypes.Fee{}, + }, + } + + _, err = decorator.AnteHandle(ctx, authtx.WrapTx(protoTx).GetTx(), false, nil) + require.ErrorContains(t, err, "EVM message contains unsupported fields") +} + +func mustNewMsgEVMTransaction(t *testing.T) *evmtypes.MsgEVMTransaction { + t.Helper() + msg, err := evmtypes.NewMsgEVMTransaction(ðtx.AssociateTx{}) + require.NoError(t, err) + return msg +} + +func wrapRawProtoTx(t *testing.T, msg *evmtypes.MsgEVMTransaction, signerInfos []*txtypes.SignerInfo, signatures [][]byte) sdk.Tx { + t.Helper() + msgAny, err := codectypes.NewAnyWithValue(msg) + require.NoError(t, err) + + protoTx := &txtypes.Tx{ + Body: &txtypes.TxBody{Messages: []*codectypes.Any{msgAny}}, + AuthInfo: &txtypes.AuthInfo{ + SignerInfos: signerInfos, + Fee: &txtypes.Fee{}, + }, + Signatures: signatures, + } + return authtx.WrapTx(protoTx).GetTx() } diff --git a/x/evm/types/message_evm_transaction.go b/x/evm/types/message_evm_transaction.go index 78e9595d6f..967eb84703 100644 --- a/x/evm/types/message_evm_transaction.go +++ b/x/evm/types/message_evm_transaction.go @@ -42,6 +42,12 @@ func (msg *MsgEVMTransaction) GetSignBytes() []byte { } func (msg *MsgEVMTransaction) ValidateBasic() error { + if msg.Derived != nil && msg.Derived.PubKey == nil { + return sdkerrors.ErrInvalidPubKey + } + if err := validateCanonicalTxData(msg.Data); err != nil { + return err + } amsg, isAssociate := msg.GetAssociateTx() if isAssociate && len(amsg.CustomMessage) > MaxAssociateCustomMessageLength { return sdkerrors.Wrapf(sdkerrors.ErrTxTooLarge, "custom message can have at most 64 characters") @@ -49,6 +55,25 @@ func (msg *MsgEVMTransaction) ValidateBasic() error { return nil } +func validateCanonicalTxData(any *codectypes.Any) error { + txData, err := UnpackTxData(any) + if err != nil { + return err + } + msg, ok := txData.(proto.Message) + if !ok { + return sdkerrors.Wrap(sdkerrors.ErrInvalidType, "invalid EVM tx data") + } + canonicalBz, err := proto.Marshal(msg) + if err != nil { + return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "could not marshal EVM tx data") + } + if len(any.Value) != len(canonicalBz) { + return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "EVM tx data contains unsupported fields") + } + return nil +} + func (msg *MsgEVMTransaction) AsTransaction() (*ethtypes.Transaction, ethtx.TxData) { txData, err := UnpackTxData(msg.Data) if err != nil { diff --git a/x/evm/types/message_evm_transaction_test.go b/x/evm/types/message_evm_transaction_test.go index c14f4f4177..ff6634cf11 100644 --- a/x/evm/types/message_evm_transaction_test.go +++ b/x/evm/types/message_evm_transaction_test.go @@ -113,3 +113,13 @@ func TestAttackerUnableToSetDerived(t *testing.T) { require.Nil(t, err) require.Equal(t, common.Address{}, decoded.Derived.SenderEVMAddr) } + +func TestValidateBasicRejectsBloatInTxData(t *testing.T) { + msg, err := types.NewMsgEVMTransaction(ðtx.AssociateTx{}) + require.NoError(t, err) + + msg.Data.Value = append(msg.Data.Value, 0x28, 0x01) + + err = msg.ValidateBasic() + require.ErrorContains(t, err, "EVM tx data contains unsupported fields") +}