hats-finance / VMEX-0x050183b53cf62bcd6c2a932632f8156953fd146f

LP token lending protocol
MIT License
2 stars 1 forks source link

might revert in an emergency #61

Open hats-bug-reporter[bot] opened 1 year ago

hats-bug-reporter[bot] commented 1 year ago

Github username: @@tamjid0x01 Submission hash (on-chain): 0xed4af40026230c137d1d43b284f3f17e573697cf79f3a5fc7d48bff5d96652b2 Severity: medium severity

Description: Description\ Funds might revert in an emergency

Attack Scenario

After depositing and withdrawing from the Aave lending pool, the current position is 0 and the strategy is in debt.

Attachments

  1. Proof of Concept (PoC) File

The main problem is that Aave lending pool doesn't allow 0 withdrawals.

    function validateWithdraw(
        address asset,
        uint64 trancheId,
        uint256 amount,
        uint256 userBalance,
        mapping(address => mapping(uint64 => DataTypes.ReserveData))
            storage reservesData,
        DataTypes.UserConfigurationMap storage userConfig,
        mapping(uint256 => address) storage reserves,
        uint256 reservesCount,
        ILendingPoolAddressesProvider _addressesProvider,
        IAssetMappings _assetMappings
    ) external {
        require(amount != 0, Errors.VL_INVALID_AMOUNT);
        require(
            amount <= userBalance,
            Errors.VL_NOT_ENOUGH_AVAILABLE_USER_BALANCE
        );

        (bool isActive, , ) = reservesData[asset][trancheId]
            .configuration
            .getFlags(asset, _assetMappings);
        require(isActive, Errors.VL_NO_ACTIVE_RESERVE);

        require(
            GenericLogic.balanceDecreaseAllowed(
                GenericLogic.BalanceDecreaseAllowedParameters(
                    asset,
                    trancheId,
                    msg.sender,
                    amount,
                    _addressesProvider,
                    _assetMappings
                ),
                reservesData,
                userConfig,
                reserves,
                reservesCount
            ),
            Errors.VL_TRANSFER_NOT_ALLOWED
        );
    }

funds might revert in an emergency if there is no position on the lending pool.

As a result, the funds might be locked inside the Contracts. I think This one an edge case.

  1. Revised Code File (Optional)

Recommended Mitigation Steps

We should check 0 withdrawal in _withdraw().


    function _withdraw(
        mapping(address => mapping(uint64 => DataTypes.ReserveData))
            storage _reserves,
        DataTypes.UserConfigurationMap storage user,
        mapping(uint256 => address) storage _reservesList,
        DataTypes.WithdrawParams memory vars,
        ILendingPoolAddressesProvider _addressesProvider,
        IAssetMappings _assetMappings
    ) external returns (uint256) {
        DataTypes.ReserveData storage reserve = _reserves[vars.asset][vars.trancheId];
        address aToken = reserve.aTokenAddress;

        uint256 userBalance = IAToken(aToken).balanceOf(msg.sender);

        if (vars.amount == type(uint256).max) {
            vars.amount = userBalance;
        }

        reserve.updateState(_assetMappings.getVMEXReserveFactor(vars.asset));

        ValidationLogic.validateWithdraw(
            vars.asset,
            vars.trancheId,
            vars.amount,
            userBalance,
            _reserves,
            user,
            _reservesList,
            vars._reservesCount,
            _addressesProvider,
            _assetMappings
        );
   if ( vars.amount != 0) {
        reserve.updateInterestRates(vars.asset, vars.trancheId, 0, vars.amount, _assetMappings.getVMEXReserveFactor(vars.asset));

         }
        if (vars.amount == userBalance) {
            user.setUsingAsCollateral(reserve.id, false);
            emit ReserveUsedAsCollateralDisabled(vars.asset, vars.trancheId, msg.sender);
        }

        IAToken(aToken).burn(
            msg.sender,
            vars.to,
            vars.amount,
            reserve.liquidityIndex
        );

        return vars.amount;
    }
ksyao2002 commented 1 year ago

Thanks for the submission. I'm not sure why not allowing zero withdraws will be a problem. Aave has not allowed zero withdraws and they have not suffered a vulnerability relating to this.

If nothing is being withdrawn, how will funds be locked in the contract?