Front-Running Risk in `enterFarm` and `enterFarmETH` Functions

[c4-bot-5]

Lines of code

https://github.com/code-423n4/2024-02-wise-lending/blob/1240a22a3bbffc13d5f8ae6300ef45de5edc7c19/contracts/PowerFarms/PendlePowerFarm/PendlePowerManager.sol#L96-L183 https://github.com/code-423n4/2024-02-wise-lending/blob/1240a22a3bbffc13d5f8ae6300ef45de5edc7c19/contracts/PowerFarms/PowerFarmNFTs/MinterReserver.sol#L77-L94

Vulnerability details

Impact

The enterFarm and enterFarmETH functions in the PendlePowerManager.sol contract generate new _keyIds and store them in the corresponding reservedKeys dictionary by calling the _reserveKey function from MinterReserver.sol.

These functions are susceptible to front-running risks.

Proof of Concept

The enterFarm and enterFarmETH functions in the PendlePowerManager.sol contract allow users to enter farming positions.

https://github.com/code-423n4/2024-02-wise-lending/blob/1240a22a3bbffc13d5f8ae6300ef45de5edc7c19/contracts/PowerFarms/PendlePowerFarm/PendlePowerManager.sol#L96-L183

    function enterFarm(
        bool _isAave,
        uint256 _amount,
        uint256 _leverage,
        uint256 _allowedSpread
    )
        external
        isActive
        updatePools
        returns (uint256)
    {
        uint256 wiseLendingNFT = _getWiseLendingNFT();

        _safeTransferFrom(
            WETH_ADDRESS,
            msg.sender,
            address(this),
            _amount
        );

        _openPosition(
            _isAave,
            wiseLendingNFT,
            _amount,
            _leverage,
            _allowedSpread
        );

        uint256 keyId = _reserveKey(
            msg.sender,
            wiseLendingNFT
        );

        isAave[keyId] = _isAave;

        emit FarmEntry(
            keyId,
            wiseLendingNFT,
            _leverage,
            _amount,
            block.timestamp
        );

        return keyId;
    }

    function enterFarmETH(
        bool _isAave,
        uint256 _leverage,
        uint256 _allowedSpread
    )
        external
        payable
        isActive
        updatePools
        returns (uint256)
    {
        uint256 wiseLendingNFT = _getWiseLendingNFT();

        _wrapETH(
            msg.value
        );

        _openPosition(
            _isAave,
            wiseLendingNFT,
            msg.value,
            _leverage,
            _allowedSpread
        );

        uint256 keyId = _reserveKey(
            msg.sender,
            wiseLendingNFT
        );

        isAave[keyId] = _isAave;

        emit FarmEntry(
            keyId,
            wiseLendingNFT,
            _leverage,
            msg.value,
            block.timestamp
        );

        return keyId;
    }

Both functions call the _reserveKey function from MinterReserver.sol to generate a new _keyId and store it in the reservedKeys dictionary.

https://github.com/code-423n4/2024-02-wise-lending/blob/1240a22a3bbffc13d5f8ae6300ef45de5edc7c19/contracts/PowerFarms/PowerFarmNFTs/MinterReserver.sol#L77-L94

    function _reserveKey(
        address _userAddress,
        uint256 _wiseLendingNFT
    )
        internal
        returns (uint256)
    {
        if (reservedKeys[_userAddress] > 0) {
            revert AlreadyReserved();
        }

        uint256 keyId = _getNextReserveKey();

        reservedKeys[_userAddress] = keyId;
        farmingKeys[keyId] = _wiseLendingNFT;

        return keyId;
    }

A potential front-running scenario could unfold as follows:

1. User A initiates a transaction attempting to reserve a _keyId by calling the enterFarm or enterFarmETH function.
2. An attacker monitoring the network detects User A's pending transaction and identifies the _keyId they are attempting to reserve.
3. The attacker constructs their own transaction, calling the same function in an attempt to reserve the _keyId, and sends this transaction with a higher gas fee.
4. If the attacker's transaction is confirmed by miners before User A's transaction, the attacker will successfully reserve the _keyId. User A's transaction will then fail due to the reservedKeys[_userAddress] > 0 check (if they use the same address) or because the attacker has already reserved the _keyId, resulting in a different _keyId for User A.

Tools Used

Manual Review

Recommended Mitigation Steps

1. Commit-Reveal Scheme: This approach can reduce the risk of front-running by having users first submit a hash ("commit") of their intention, followed by a subsequent transaction to reveal their intent ("reveal"). This ensures that attackers cannot determine the user's actual intent during the "commit" phase.

2. Access Control: Ensure that only verified or authorized users can reserve _keyIds. Although this does not eliminate front-running entirely, it restricts who can perform the operation.

Assessed type

Timing

[GalloDaSballo]

Unclear if this has any real impact

[c4-pre-sort]

GalloDaSballo marked the issue as insufficient quality report

[c4-pre-sort]

GalloDaSballo marked the issue as primary issue

[vm06007]

4. If the attacker's transaction is confirmed by miners before User A's transaction, the attacker will successfully reserve the _keyId. User A's transaction will then fail due to the reservedKeys[_userAddress] > 0 check (if they use the same address) or because the attacker has already reserved the _keyId, resulting in a different _keyId for User A.

this is not correct assumption, misleading this and blowing it over proportion.

• if someone's transaction is accepted earlier, this means that next transaction will receive next key, there is no failure or ability to deny something with front-run on entering farm, since keyId is not a parameter, it is something that gets issued so should there be even 10 transaction in each block they do not collide one with another due to keyIds, As long as there's enough funds all will be successful.

[c4-judge]

trust1995 marked the issue as unsatisfactory: Invalid