Aura Finance contest - Aits's results

Lines of code

https://github.com/code-423n4/2022-05-aura/blob/main/contracts/ExtraRewardsDistributor.sol#L93 https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/contracts/AuraClaimZap.sol#L171-L215

Vulnerability details

Impact

There are ERC20 tokens that may make certain customizations to their ERC20 contracts. One type of these tokens is deflationary tokens that charge a certain fee for every safetransfer() or safetransferFrom() .

Proof of Concept

The ExtraRewardsDistributor’s deposit() ,_addRewardand _claimExtras functions transfer _amount to this contract using IERC20(_token).safeTransferFrom(msg.sender, address(this), _amount);. This could have a fee, and less than _amount ends up in the contract. The next actual vault deposit using

 ICrvDepositorWrapper(crvDepositWrapper).deposit( crvBalance,minAmountOut, _checkOption(options, uint256(Options.LockCrvDeposit)), address(0); 

will then try to transfer more than the this contract actually has and will revert the transaction.

AuraClaimZap.sol#L171-L215

    function _claimExtras( // solhint-disable-line 
        uint256 depositCrvMaxAmount,
        uint256 minAmountOut,
        uint256 depositCvxMaxAmount,
        uint256 removeCrvBalance,
        uint256 removeCvxBalance,
        uint256 options
    ) internal {
        //claim from cvxCrv rewards
        if (_checkOption(options, uint256(Options.ClaimCvxCrv))) {
            IBasicRewards(cvxCrvRewards).getReward(msg.sender, true);
        }

        //claim from locker
        if (_checkOption(options, uint256(Options.ClaimLockedCvx))) {
            IAuraLocker(locker).getReward(msg.sender, _checkOption(options, uint256(Options.ClaimLockedCvxStake)));
        }

        //reset remove balances if we want to also stake/lock funds already in our wallet
        if (_checkOption(options, uint256(Options.UseAllWalletFunds))) {
            removeCrvBalance = 0;
            removeCvxBalance = 0;
        }

        //lock upto given amount of crv and stake
        if (depositCrvMaxAmount > 0) {
            uint256 crvBalance = IERC20(crv).balanceOf(msg.sender).sub(removeCrvBalance);
            crvBalance = AuraMath.min(crvBalance, depositCrvMaxAmount);

            if (crvBalance > 0) {
                //pull crv
                IERC20(crv).safeTransferFrom(msg.sender, address(this), crvBalance);
                //deposit
                ICrvDepositorWrapper(crvDepositWrapper).deposit(
                    crvBalance,
                    minAmountOut,
                    _checkOption(options, uint256(Options.LockCrvDeposit)),
                    address(0)
                );

                uint256 cvxCrvBalance = IERC20(cvxCrv).balanceOf(address(this));
                //stake for msg.sender
                IBasicRewards(cvxCrvRewards).stakeFor(msg.sender, cvxCrvBalance);
            }
        }

Tools Used

Manual Test

Recommended Mitigation Steps

One possible mitigation is to measure the asset change right before and after the asset-transferring routines.