From 046e3860efca75032156573246505b8c072dda91 Mon Sep 17 00:00:00 2001 From: Ardit Marku Date: Wed, 26 Jun 2024 15:43:00 +0300 Subject: [PATCH] Validate TransactionArgs passed in eth_estimateGas & eth_call JSON-RPC endpoints --- api/api.go | 10 +++ api/models.go | 74 ++++++++++++++++++++++ api/models_test.go | 149 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 233 insertions(+) create mode 100644 api/models_test.go diff --git a/api/api.go b/api/api.go index 68ada9ac0..adb178c75 100644 --- a/api/api.go +++ b/api/api.go @@ -487,6 +487,11 @@ func (b *BlockChainAPI) Call( return nil, err } + err := args.Validate() + if err != nil { + return nil, err + } + evmHeight, err := b.getBlockNumber(blockNumberOrHash) if err != nil { return handleError[hexutil.Bytes](b.logger, err) @@ -628,6 +633,11 @@ func (b *BlockChainAPI) EstimateGas( return 0, err } + err := args.Validate() + if err != nil { + return 0, err + } + tx, err := encodeTxFromArgs(args) if err != nil { return hexutil.Uint64(blockGasLimit), nil // return block gas limit diff --git a/api/models.go b/api/models.go index 860d6f81c..705c87e3c 100644 --- a/api/models.go +++ b/api/models.go @@ -1,6 +1,11 @@ package api import ( + "bytes" + "errors" + "fmt" + "math/big" + errs "github.com/onflow/flow-evm-gateway/api/errors" "github.com/onflow/flow-evm-gateway/config" "github.com/onflow/flow-evm-gateway/models" @@ -33,6 +38,75 @@ type TransactionArgs struct { ChainID *hexutil.Big `json:"chainId,omitempty"` } +func (txArgs TransactionArgs) Validate() error { + // Prevent accidental erroneous usage of both 'input' and 'data' (show stopper) + if txArgs.Data != nil && txArgs.Input != nil && !bytes.Equal(*txArgs.Data, *txArgs.Input) { + return errors.New(`ambiguous request: both "data" and "input" are set and are not identical`) + } + + // Place data on 'data' + var data []byte + if txArgs.Input != nil { + txArgs.Data = txArgs.Input + } + if txArgs.Data != nil { + data = *txArgs.Data + } + + txDataLen := len(data) + + // Contract creation doesn't validate call data, handle first + if txArgs.To == nil { + // Contract creation should contain sufficient data to deploy a contract. A + // typical error is omitting sender due to some quirk in the javascript call + // e.g. https://github.com/onflow/go-ethereum/issues/16106. + if txDataLen == 0 { + // Prevent sending ether into black hole (show stopper) + if txArgs.Value.ToInt().Cmp(big.NewInt(0)) > 0 { + return errors.New("transaction will create a contract with value but empty code") + } + + // No value submitted at least, critically Warn, but don't blow up + return errors.New("transaction will create a contract with empty code") + } + + if txDataLen < 40 { // arbitrary heuristic limit + return fmt.Errorf( + "transaction will create a contract, but the payload is suspiciously small (%d bytes)", + txDataLen, + ) + } + } + + // Not a contract creation, validate as a plain transaction + if txArgs.To != nil { + to := common.NewMixedcaseAddress(*txArgs.To) + if !to.ValidChecksum() { + return errors.New("invalid checksum on recipient address") + } + + if bytes.Equal(to.Address().Bytes(), common.Address{}.Bytes()) { + return errors.New("transaction recipient is the zero address") + } + + // If the data is not empty, validate that it has the 4byte prefix and the rest divisible by 32 bytes + if txDataLen > 0 { + if txDataLen < 4 { + return errors.New("transaction data is not valid ABI (missing the 4 byte call prefix)") + } + + if n := txDataLen - 4; n%32 != 0 { + return fmt.Errorf( + "transaction data is not valid ABI (length should be a multiple of 32 (was %d))", + n, + ) + } + } + } + + return nil +} + // AccessListResult returns an optional accesslist // It's the result of the `debug_createAccessList` RPC call. // It contains an error if the transaction itself failed. diff --git a/api/models_test.go b/api/models_test.go new file mode 100644 index 000000000..6f502c8f6 --- /dev/null +++ b/api/models_test.go @@ -0,0 +1,149 @@ +package api + +import ( + "encoding/hex" + "math/big" + "testing" + + "github.com/onflow/go-ethereum/common" + "github.com/onflow/go-ethereum/common/hexutil" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestValidateTransaction(t *testing.T) { + validToAddress := common.HexToAddress("0x000000000000000000000000000000000000dEaD") + zeroToAddress := common.HexToAddress("0x0000000000000000000000000000000000000000") + + data, err := hex.DecodeString("c6888fa1") + require.NoError(t, err) + smallContractPayload := hexutil.Bytes(data) + + data, err = hex.DecodeString("c6888f") + require.NoError(t, err) + invalidTxData := hexutil.Bytes(data) + + data, err = hex.DecodeString("c6888fa1000000000000000000000000000000000000000000000000000000000000ab") + require.NoError(t, err) + invalidTxDataLen := hexutil.Bytes(data) + + gasLimit := uint64(125_000) + + nonce := uint64(1) + tests := map[string]struct { + txArgs TransactionArgs + valid bool + errMsg string + }{ + "valid transaction": { + txArgs: TransactionArgs{ + Nonce: (*hexutil.Uint64)(&nonce), + To: &validToAddress, + Value: (*hexutil.Big)(big.NewInt(0)), + Gas: (*hexutil.Uint64)(&gasLimit), + GasPrice: (*hexutil.Big)(big.NewInt(0)), + Data: &hexutil.Bytes{}, + }, + valid: true, + errMsg: "", + }, + "conflicting input and data": { + txArgs: TransactionArgs{ + Nonce: (*hexutil.Uint64)(&nonce), + To: &validToAddress, + Value: (*hexutil.Big)(big.NewInt(0)), + Gas: (*hexutil.Uint64)(&gasLimit), + GasPrice: (*hexutil.Big)(big.NewInt(0)), + Data: &invalidTxDataLen, + Input: &invalidTxData, + }, + valid: false, + errMsg: `ambiguous request: both "data" and "input" are set and are not identical`, + }, + "send to 0 address": { + txArgs: TransactionArgs{ + Nonce: (*hexutil.Uint64)(&nonce), + To: &zeroToAddress, + Value: (*hexutil.Big)(big.NewInt(0)), + Gas: (*hexutil.Uint64)(&gasLimit), + GasPrice: (*hexutil.Big)(big.NewInt(0)), + Data: &hexutil.Bytes{}, + }, + valid: false, + errMsg: "transaction recipient is the zero address", + }, + "create empty contract (no value)": { + txArgs: TransactionArgs{ + Nonce: (*hexutil.Uint64)(&nonce), + To: nil, + Value: (*hexutil.Big)(big.NewInt(0)), + Gas: (*hexutil.Uint64)(&gasLimit), + GasPrice: (*hexutil.Big)(big.NewInt(0)), + Data: &hexutil.Bytes{}, + }, + valid: false, + errMsg: "transaction will create a contract with empty code", + }, + "create empty contract (with value)": { + txArgs: TransactionArgs{ + Nonce: (*hexutil.Uint64)(&nonce), + To: nil, + Value: (*hexutil.Big)(big.NewInt(150)), + Gas: (*hexutil.Uint64)(&gasLimit), + GasPrice: (*hexutil.Big)(big.NewInt(0)), + Data: &hexutil.Bytes{}, + }, + valid: false, + errMsg: "transaction will create a contract with value but empty code", + }, + "small payload for create": { + txArgs: TransactionArgs{ + Nonce: (*hexutil.Uint64)(&nonce), + To: nil, + Value: (*hexutil.Big)(big.NewInt(150)), + Gas: (*hexutil.Uint64)(&gasLimit), + GasPrice: (*hexutil.Big)(big.NewInt(0)), + Data: &smallContractPayload, + }, + valid: false, + errMsg: "transaction will create a contract, but the payload is suspiciously small (4 bytes)", + }, + "tx data length < 4": { + txArgs: TransactionArgs{ + Nonce: (*hexutil.Uint64)(&nonce), + To: &validToAddress, + Value: (*hexutil.Big)(big.NewInt(150)), + Gas: (*hexutil.Uint64)(&gasLimit), + GasPrice: (*hexutil.Big)(big.NewInt(0)), + Data: &invalidTxData, + }, + valid: false, + errMsg: "transaction data is not valid ABI (missing the 4 byte call prefix)", + }, + "tx data (excluding function selector) not divisible by 32": { + txArgs: TransactionArgs{ + Nonce: (*hexutil.Uint64)(&nonce), + To: &validToAddress, + Value: (*hexutil.Big)(big.NewInt(150)), + Gas: (*hexutil.Uint64)(&gasLimit), + GasPrice: (*hexutil.Big)(big.NewInt(0)), + Data: &invalidTxDataLen, + }, + valid: false, + errMsg: "transaction data is not valid ABI (length should be a multiple of 32 (was 31))", + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + err := tc.txArgs.Validate() + if tc.valid { + require.NoError(t, err) + } else { + require.Error(t, err) + assert.ErrorContains(t, err, tc.errMsg) + } + }) + } + +}