PoolTogether TWAB Delegator contest - cmichel's results

Lines of code

https://github.com/pooltogether/v4-twab-delegator/blob/2b6d42506187dd7096043e2dfec65fa06ab18577/contracts/TWABDelegator.sol#L442

Vulnerability details

Impact

The permitAndMulticall function does not authenticate the _data argument through any signatures. Only the _from, _amount parameters are as chosen by the _from user as these are checked in the _permitAndMulticall -> ERC20(ticket).permit() call. An attacker can frontrun a permitAndMulticall transaction and inject different _data that is then executed.

Luckily, this doesn't lead to loss of funds as all transferFrom() calls in TWABDelegator still use msg.sender as the sender and any such transfer would fail for the frontrunner as frontrunner=msg.sender != _from.

However, they can frontrun and submit an empty _data array, only performing the token approval for the _from user. The original transaction will then fail in the ERC20.permit as the nonce is already used, breaking the core permitAndMulticall use case.

Recommended Mitigation Steps

Make sure that the _data matches the data chosen by the original from user. Either, by adding a second signature from from for _data. Alternatively, as it's implicitly assumed that _from = msg.sender (otherwise the transferFroms will fail), add an explicit require(_from == msg.sender) in permitAndMulticall.