Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
1
The
postRelayedCall
function of theTokenPaymaster
contract is public and allows anyone to call it. In this function the uniswap router is called and leftover funds are sent back to the user. Thus, this function modifies the state. Without an access control check that only allows theRelayHub
to call it, an attacker can craft call data that will drain the ERC20 balance of the contract. Note that normally the contract is not supposed to have an ERC20 balance, however, this can change in situations as described in Anyone can steal GSN fees.The
preRelayedCall
andpostRelayedCall
functions both have following comment in the official GSNIPaymaster
class:https://github.com/poanetwork/tokenbridge-contracts/blob/b3511bf0987bbfef661e28dd1a6fbe1735f90ac0/contracts/gsn/interfaces/IPaymaster.sol#L57
The
preRelayedCall
function does not modify state but it could still make sense to add an access control check.2
The contracts use an outdated GSN interface in the definition of
IPaymaster
. In particular the following differences exist:getGasLimits
should now be calledgetGasAndDataLimits()
and should return four instead of three values.trustedForwarder
function should be implemented.Furthermore, the
RelayData
struct has changed. As less context information is available the security guarantees are weaker.The updates in this PR consist of:
relayHubOnly
modifier forpostRelayedCall
GasAndDataLimits
struct andtrustedForwarder
function inIPaymaster
interfaceRelayData
structclaimTokens
function inTokenPaymaster