Lambda - CardTopupPermit calls can be griefed

[sherlock-admin]

Lambda

medium

CardTopupPermit calls can be griefed

Summary

An attacker can frontrun all CardTopupPermit calls to make topping up the card impossible for a user.

Vulnerability Detail

When an attacker sees a CardTopupPermit call in the mempool, he can extract the permit data from it, and call permit on the token itself with the provided data. Note that this works because any user can call permit for a different user given the signature (see e.g. https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC20/extensions/draft-ERC20Permit.sol#L49). Of course, it would not work when the attacker calls CardTopupPermit (because msg.sender is passed to _processTopup), as this function would revert.

Impact

When the permit call was frontrun, the second call (in CardTopupPermit) will revert, because the nonce was already used. Therefore, the topup of the user fails. Note that an attacker can repeat this attack multiple times to launch a DoS on a user and prevent him from ever using CardTopupPermit to top up his card.

Code Snippet

https://github.com/sherlock-audit/2022-10-mover/blob/main/cardtopup_contract/contracts/HardenedTopupProxy.sol#L1010

Tool used

Manual Review

Recommendation

In CardTopupPermit, it should be checked if the approval is already equal to the desired value (which would be the case when these calls are frontrun). Then, permit should not be called.

[McMannaman]

it should be checked if the approval is already equal to the desired value

It is checked after the permit. However, it is checked in the other execution paths too, for a specific reason that allowance/permit is recent enough.

Then, permit should not be called.

this is why we intentionally want to permit to be called. Works as expected. If someone wants to block user from using permit it could be done for any contract that is expecting permit data. Out of scope and not an issue.