HardhatDeFi
15,000 USDC
View results
Submission Details
Severity: high
Invalid

Missing zero value checks may leads to potential DOS & un-necessary gas spending for the protocol.

Summary

In AaveDIVAWrapper.sol & AaveDIVAWrapperCore.sol there are multiple functions which lacks on checking zero input value being passed while calling the function. There are multiple functions which have this issue which will leads to multiple call being made resulting in un-necessary consuming gas which will lead to DOS for the protocol .

Vulnerability Details

There are multiple function's which take input parameters from users and didn't check for zero value being used which will make protocol spend un-necessary gas fees for calling external call's .

Functions which didn't check for zero value is :: _createContingentPool(), _addLiquidity(), _removeLiquidity(), _redeemPositionToken(), _redeemWToken().

  • Let's take one of the function and go throught the transaction flow ::

    • Suppose Attacker calls AaveDIVAWrapper.sol::redeemPositionToken() it will calls AaveDIVAWrapperCore.sol::_redeemPositionToken.

    • Let's take _positionTokenAmount passed by Attacker in input params is zero :: _positionTokenAmount = 0

    • After calling the function now it will fetch the pool parameters using the poolID which is being passed in input parameters.

    • Attacker holds zero balance of _positionToken .

    • Now this function will transfer positionToken from attacker to AAveDivaWrapper contract .

    // Transfer position token (long or short) from caller to this contract. Requires prior approval from the caller
    // to transfer the position token to this contract.
    // No need to use `safeTransferFrom` here as position tokens in DIVA Protocol are standard ERC20 tokens
    // using OpenZeppelin's ERC20 implementation.
    _positionTokenContract.transferFrom(
    msg.sender /** from */,
    address(this) /** to */,
    _positionTokenAmountToRedeem
    );
    • The call will be succeded as the attacker has transferred zero tokens and Openzeppelin ERC20 didn't revert on zero amount transfers.

    • Now this function will make a call from DIVA contract to redeemPositionToken with same zero amount And that call also didn't revert because there is no check there also in the DIVA contract.

    // Redeem position token on DIVA Protocol to receive wTokens, and calculate the returned wToken amount (net of DIVA fees)
    // as DIVA Protocol's redeemPositionToken function does not return the amount of collateral token received.
    uint256 _wTokenBalanceBeforeRedeem = _collateralTokenContract.balanceOf(address(this));
    // @audit :: This function also didn;t check for zero amount and it will pass with the zero token with successful transfer
    IDIVA(_diva).redeemPositionToken(_positionToken, _positionTokenAmountToRedeem);
    uint256 _wTokenAmountReturned = _collateralTokenContract.balanceOf(address(this)) - _wTokenBalanceBeforeRedeem;
    • Now it's time to withdraw collateral token from Aave [ Note :: The attacker didn't have any token As he didn't add any liquity].

    • Now this function will make a call to _redeemWTokenPrivate() it will burn wToken as ERC20 allows to burn zero token it will not revert .

    • Now when the call is been made to AAVEv3Pool to withdraw then this function will get revert as the Aave withdraw function checks that the amount is not zero if zero it will revert.

  • Similarly other functions can also be reverted only when they make a call to AAve contract till then all calls will be executed with zero value without being reverted.

After making multiple successful external calls inside a function until it reverts while calling AaveV3 function's . So the external calls which has been made before reverting consumes gas as they were successful call/transaction. In that way the protocol has to pay gas fees and Similarly Attacker can make multiple calls which will create a situation of DOS it will result in increase of gas fees over a protocol and also a loss of funds in terms of fees for the protocol.

Impact

Impact :: HIGH
Likelihood :: HIGH
Exploiting this issue by calling function multiple time with zero amount leads to potential DOS and un-necessary gas spending for the Protocol.

  • Loss of Funds in terms of un-necessary gas spending over external calls before reverting.

Tools Used

Manual Testing

Recommendations

Simply adding amount check which will check that the input parameter didn't have zero amount input will solve the issue. If there is zero amount revert the call.
For eg. In _redeemPositionToken() function add

require(_positionTokenAmount != 0, "Amount cannot be zero").
Updates

Lead Judging Commences

bube Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Appeal created

krot0025 Submitter
9 months ago
bube Lead Judge
9 months ago
bube Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.