(I privately contacted the authors about a vulnerability in the implementation, and have approval to post it here.)
Thanks for this proposal. I found it while investigating possible alternatives to Permit2-type signature schemes. I can see the UX benefits of avoiding the need for authorisations, and the security benefits of being able to show and explicitly guarantee the token outputs of a transaction in a wallet before sending. Although I fear that there is a chicken-and-egg situation here, that unless the scheme has support from wallets to visualise the transaction input and output constraints, users won’t be able to understand what they’re guaranteeing, which could result in them accepting transactions without understanding the implications anyway. And wallets probably need user adoption before they would invest the effort to visualise this kind of interaction.
This makes me wonder if there is, or we collectively need a way to create shared, approved visualisations of contract calls (and likewise for structured signatures), so that wallets could have a generic way of showing a reasonable representation of an action without needing to hand-craft a special case for every call.
More significantly though, while reading the example implementation, I noticed a vulnerability that allows a contract to withdraw funds held by the router during a call without authorisation.
The router returns unspent funds to the caller, but doesn’t check if a call is reentrant and doesn’t segregate the funds of concurrent calls, so a reentrant
exec() call can transfer 0 tokens into the router and the router will refund the reentrant call funds deposited into the router from the outer call.
So if a trusted protocol intentionally executed by the router interacts with any untrusted code, that untrusted code could call back into the router to withdraw funds it wan’t intended to have access to.
I have repo of the issue here: https://github.com/h4l/erc-6120-poc/blob/main/test/UniversalTokenRouter.t.sol
As well as ERC-20s and Eth, this also applies to NFT tokens, but I didn’t implement reproductions for those.
The issue could be mitigated by tight use of the output enforcement feature your contract provides, which does demonstrate the strength of this kind of approach. But I expect that in practice it is not possible to precisely define the exact outputs, whether because the exact result is not known, or because the transaction is one-way, e.g. depositing funds without immediately receiving something equivalent. So a malicious contract could skim off a portion of funds without being caught, or could do things like swap one NFT for another inside a transaction, which may not be caught by output constraints for NFT counts. (e.g. when minting an NFT, the
tokenId is not always known, so the minted token could be swapped out in the router by an untrusted contract — a rare one for a common one for example.)