The permit() ERC20/ERC721 extension (EIP2612, EIP4494) is widely used to save users the hassle of sending an approve() TX before a contract interaction. The controller uses them.
The issue with the permit() call implementation is that anyone observing the mempool is able to DOS the controller transaction. They would copy the permit params and call permit() directly on the ERC20/ERC721 contract. This would advance the permit nonce and when permit() will try authenticating the signature, it would revert.
The DOS is not permanent because a user could pass an empty PermitParams after it was called by an attacker, in this case the call to permit() is skipped.
Recommended mitigation
Wrap the permit() external call with a try/catch. In case of failure, if the user allowance is
sufficient, do not revert.
Description
The permit() ERC20/ERC721 extension (EIP2612, EIP4494) is widely used to save users the hassle of sending an approve() TX before a contract interaction. The controller uses them.
https://github.com/clober-dex/coupon-finance/blob/216e0d9463b340ffcc810ecc81957ebe309048ec/contracts/libraries/Controller.sol#L147-L163
The issue with the permit() call implementation is that anyone observing the mempool is able to DOS the controller transaction. They would copy the permit params and call permit() directly on the ERC20/ERC721 contract. This would advance the permit nonce and when permit() will try authenticating the signature, it would revert. The DOS is not permanent because a user could pass an empty PermitParams after it was called by an attacker, in this case the call to permit() is skipped.
Recommended mitigation
Wrap the permit() external call with a try/catch. In case of failure, if the user allowance is sufficient, do not revert.