c4-bot-9 commented 8 months ago

Lines of code

https://github.com/limitbreakinc/PermitC/blob/c431dc5e80690c1d8c3727f5992d519df3d38254/src/PermitC.sol#L783-L798

Vulnerability details

Impact

While a great effort was made to mitigate out of order nonce execution

The reality is that since PermitC allows unordered nonces, then nonces could be executed out of order

This opens up more problems than the original Permit Exploit

The original Permit exploit allowed to effectively always make multi-operations revert

This new scenario instead allows to use the order of nonces that will cause the most damage

The most basic example would be using the reverse order to keep allowances to be non-zero

This may open up to more exploits, an example being claiming TAP, twTAP or rewards to router addresses by keeping the allowance as non-zero

Additionally, any time more than one Permit is issued, a race condition is available

This race condition would allow an exploiter to consume the nonces out of order, and then cause the transaction to revert

This, in contrast to the previous iteration allows all possible sequences of combinations of allowances to determine the final result, while the original permit operation would allow only the proper sequence to be executed (with pauses between each step)

This additional risk can create scenarios in which outstanding allowance is left, which, for example, could incorrectly signal the willingness to claim tokens or the willingness to allow a target to bridge funds at a different time than intended

It's worth reiterating how Signatures can be executed as soon as made available on any mempool, whether a L2 or LayerZeros system

POC

Victim signs Permit A, B and C
Notice how any of the permutations of [A, B, C] could be used out of order
Front-runner will execute those signatures at whichever order is best for them to leak the most value

Mitigation

The only solution I have found is to ensure that all "Macros" in Magnetar always approve a specific contract at most once

And that revokes are done without signatures, either by consuming the allowance via a transferFrom (which may not be possible) or by introducing a function that allows an operator to reset their allowance

Assessed type

MEV

c4-judge commented 8 months ago

dmvt marked the issue as primary issue

c4-sponsor commented 8 months ago

0xRektora (sponsor) acknowledged

0xRektora commented 8 months ago

If I remember correctly, the team from Limit Break did not build PermitC to have batched transaction, but it can be dangerous if implemented wrong. Will forward the message. As for Tapioca, we do have a batch function in Pearlmit that will force the signatures to be verified against the order they were sent to. We believe the solution to the griefing scenario would be to use something like permitTransferFromWithAdditionalDataERC20 on said batch, by effectively binding the whole Tx (approvals paired with compose messages) and force the approval to be executed only in the case, which would nullify the front-running incentives.

0xRektora commented 8 months ago

Limit Break answer --

Yes - as designed it's explicity allowed to use unordered nonces, so this would be valid. A modification could be made to the _checkAndInvalidateNonce function if you want to do ordered where you have a tracker by account and make sure that nonce == nextNonce[owner] but we don't want that for our base case as unordered nonces are required to execute orders out of sequence