Skip to content

Commit

Permalink
provider: reject toxic transaction requests
Browse files Browse the repository at this point in the history
  • Loading branch information
attente committed Jul 27, 2023
1 parent cc066e6 commit 6c4b7a9
Show file tree
Hide file tree
Showing 3 changed files with 185 additions and 4 deletions.
30 changes: 30 additions & 0 deletions packages/provider/src/transactions.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import { commons } from '@0xsequence/core'
import { ethers } from 'ethers'

export function validateTransactionRequest(wallet: string, transaction: commons.transaction.Transactionish) {
const transactions = commons.transaction.fromTransactionish(wallet, transaction)
const unwound = commons.transaction.unwind(wallet, transactions)
unwound.forEach(transaction => validateTransaction(wallet, transaction))
}

function validateTransaction(wallet: string, transaction: commons.transaction.Transaction) {
if (transaction.to.toLowerCase() === wallet.toLowerCase()) {
if (transaction.data) {
const data = ethers.utils.arrayify(transaction.data)
if (data.length >= 4) {
const selector = ethers.utils.hexlify(data.slice(0, 4))
switch (selector) {
case '0x7a9a1628': // execute((bool,bool,uint256,address,uint256,bytes)[],uint256,bytes)
case '0x61c2926c': // selfExecute((bool,bool,uint256,address,uint256,bytes)[])
break
default:
throw new Error('self calls other than execute and selfExecute are forbidden')
}
}
}
}

if (transaction.delegateCall) {
throw new Error('delegate calls are forbidden')
}
}
13 changes: 9 additions & 4 deletions packages/provider/src/transports/wallet-request-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { BigNumber, ethers, providers } from 'ethers'
import { EventEmitter2 as EventEmitter } from 'eventemitter2'

import { fromExtended } from '../extended'
import { validateTransactionRequest } from '../transactions'
import {
ProviderMessageRequest,
ProviderMessageResponse,
Expand Down Expand Up @@ -424,6 +425,8 @@ export class WalletRequestHandler implements ExternalProvider, JsonRpcHandler, P
return tx
})

validateTransactionRequest(account.address, transactionParams)

let txnHash = ''
if (this.prompter === null) {
// prompter is null, so we'll send from here
Expand Down Expand Up @@ -471,12 +474,14 @@ export class WalletRequestHandler implements ExternalProvider, JsonRpcHandler, P
// and would have prompted the user upon signing.

// https://eth.wiki/json-rpc/API#eth_sendRawTransaction
if (commons.transaction.isSignedTransactionBundle(request.params![0])) {
const txChainId = BigNumber.from(request.params![0].chainId).toNumber()
const tx = await account.relayer(txChainId)!.relay(request.params![0])
const transaction = request.params![0]
if (commons.transaction.isSignedTransactionBundle(transaction)) {
validateTransactionRequest(account.address, transaction.transactions)
const txChainId = BigNumber.from(transaction.chainId).toNumber()
const tx = await account.relayer(txChainId)!.relay(transaction)
response.result = tx.hash
} else {
const tx = await provider.sendTransaction(request.params![0])
const tx = await provider.sendTransaction(transaction)
response.result = tx.hash
}
break
Expand Down
146 changes: 146 additions & 0 deletions packages/provider/tests/transactions.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
import { commons } from '@0xsequence/core'
import { expect } from 'chai'
import { validateTransactionRequest } from '../src/transactions'

const self = '0x5e1f5e1f5e1f5e1f5e1f5e1f5e1f5e1f5e1f5e1f'
const to = '0x0123456789012345678901234567890123456789'

describe('validating transaction requests', () => {
it('should throw an error when a transaction does a self call', () => {
const transaction = {
to,
data: commons.transaction.encodeBundleExecData({
entrypoint: to,
transactions: [
{
to: self,
data: '0x12345678'
}
]
})
}

expect(() => validateTransactionRequest(self, transaction)).to.throw()
})

it('should throw an error when a transaction does a deep self call', () => {
const transaction = {
to,
data: commons.transaction.encodeBundleExecData({
entrypoint: to,
transactions: [
{
to: self,
data: commons.transaction.encodeBundleExecData({
entrypoint: self,
transactions: [
{
to: self,
data: '0x12345678'
}
]
})
}
]
})
}

expect(() => validateTransactionRequest(self, transaction)).to.throw()
})

it('should throw an error when a transaction does a delegate call', () => {
const transaction = {
to,
data: commons.transaction.encodeBundleExecData({
entrypoint: to,
transactions: [
{
to,
delegateCall: true
}
]
})
}

expect(() => validateTransactionRequest(self, transaction)).to.throw()
})

it('should throw an error when a transaction does a deep delegate call', () => {
const transaction = {
to,
data: commons.transaction.encodeBundleExecData({
entrypoint: to,
transactions: [
{
to: self,
data: commons.transaction.encodeBundleExecData({
entrypoint: self,
transactions: [
{
to: self,
delegateCall: true
}
]
})
}
]
})
}

expect(() => validateTransactionRequest(self, transaction)).to.throw()
})

it('should succeed when a transaction does only a deep execute call', () => {
const transaction = {
to,
data: commons.transaction.encodeBundleExecData({
entrypoint: to,
transactions: [
{
to: self,
data: commons.transaction.encodeBundleExecData({
entrypoint: self,
transactions: [
{
to: self,
value: '1000000000000000000'
}
]
})
}
]
})
}

expect(() => validateTransactionRequest(self, transaction)).to.not.throw()
})

it('should not throw an error in general', () => {
const transaction = {
to,
data: commons.transaction.encodeBundleExecData({
entrypoint: to,
transactions: [
{
to: self, // self without data is ok
value: '1000000000000000000'
},
{
to: self, // execute is ok
data: commons.transaction.encodeBundleExecData({
entrypoint: self,
transactions: [
{
to,
data: '0x12345678'
}
]
})
}
]
})
}

expect(() => validateTransactionRequest(self, transaction)).to.not.throw()
})
})

0 comments on commit 6c4b7a9

Please sign in to comment.