Matin - ERC20 permit() calls can be front-ran and user deposit tasks can be DoSed

[sherlock-admin2]

Matin

Medium

ERC20 permit() calls can be front-ran and user deposit tasks can be DoSed

Summary

The permit() functionality which is used inside the market and user manager contracts, lacks protection against front-running attacks, which can lead to a denial of service (DoS) for user deposits.

Vulnerability Detail

The permit() function verifies if the provided signature matches the owner or signer of the message but does not check who executes the permit(). Thus, Alice can grant a permit to Bob, but John could use it on behalf of Alice or Bob. While not inherently problematic, this becomes an issue in transaction call chains susceptible to front-running. If a permit call is front-ran, the transaction will revert because using an already utilized signature will fail.

The issue exists within the market and user manager contracts that enable the permit() feature, allowing users to perform important tasks like staking and borrow repayment using permits.

In the context of staking with a permit: (Staking among other tasks is selected for illustration)

1. Alice grants Bob permission to stake tokens on her behalf.
2. Bob calls stakeWithERC20Permit(), providing the permits, which calls permit() to execute the permit and complete the staking.

However, when Alice grants these permits to Bob and he tries to execute them, the signature details are exposed to the mempool. A malicious user can:

1. See the permit signature parameters provided by Bob.
2. Front-run the stakeWithERC20Permit() call, directly call ERC20 permit(), and use the signature.

Bob's attempt to stake on Alice's behalf would then fail because the signature he provided is already used, causing a revert when he reaches permit().

For more information, refer to this article.

Impact

The permit() function does not verify the executor's identity, enabling front-running attacks. Malicious users can exploit this by using exposed permit signatures from the mempool, causing legitimate transactions to revert. This can disrupt processes like staking with permits, leading to denial of service and wasted gas fees for users.

Code Snippet

https://github.com/sherlock-audit/2024-06-union-finance-update-2/blob/main/union-v2-contracts/contracts/user/UserManagerERC20.sol#L19-L30
https://github.com/sherlock-audit/2024-06-union-finance-update-2/blob/main/union-v2-contracts/contracts/user/UserManagerDAI.sol#L20-L33
https://github.com/sherlock-audit/2024-06-union-finance-update-2/blob/main/union-v2-contracts/contracts/user/UserManager.sol#L703-L713
https://github.com/sherlock-audit/2024-06-union-finance-update-2/blob/main/union-v2-contracts/contracts/market/UDai.sol#L9-L24
https://github.com/sherlock-audit/2024-06-union-finance-update-2/blob/main/union-v2-contracts/contracts/market/UErc20.sol#L8-L22

Proof of Concept

    function testRevert_SignatureReplay() public {
        SigUtils.Permit memory permit = SigUtils.Permit({
            owner: owner,
            spender: spender,
            value: 1e18,
            nonce: 0,
            deadline: 1 days
        });

        bytes32 digest = sigUtils.getTypedDataHash(permit);

        (uint8 v, bytes32 r, bytes32 s) = vm.sign(ownerPrivateKey, digest);

        token.permit(
            permit.owner,
            permit.spender,
            permit.value,
            permit.deadline,
            v,
            r,
            s
        );

        uint amount = 5e17;

        vm.expectRevert();
        userManager.stakeWithPermit(
            amount,
            permit.nonce,
            permit.deadline,
            v,
            r,
            s
        );
    }

Tool used

Manual Review

Recommendation

Similar to the solution proposed in the linked article, wrap each permit() call in a try/catch block. This will prevent already granted permissions from causing the entire call chain to revert, allowing the transaction to proceed and use the granted allowance as intended.

Duplicate of #65