sherlock-audit / 2023-12-flatmoney-judging

11 stars 9 forks source link

ravikiran.web3 - StableModule::executeDeposit() can face Denial of Services preventing deposits temporarily and at unpredictable instances of time #53

Closed sherlock-admin closed 8 months ago

sherlock-admin commented 8 months ago

ravikiran.web3

medium

StableModule::executeDeposit() can face Denial of Services preventing deposits temporarily and at unpredictable instances of time

Summary

executeDeposit() function could revert due to division by zero error. While the possibility exists, the circumstances for its occurrence are erratic and random. The problem will occur when totalAfterSettlement results in a value less than 0. This will result in stableCollateralPerShare() being returned as 0 resulting in error during executeDeposit() call.

While the problematic circumstances remains, the executeDeposit() will revert for the calls.

Vulnerability Detail

To understand the problems, below is the path of control flow for the function.

a) StableModule::executeDeposit() The executeDeposit() function computes _liquidityMinted by reading the collateral per share as below. The problem occurs if stableCollateralPerShare(maxAge) was returned as 0.

    _liquidityMinted = (depositAmount * (10 ** decimals())) / stableCollateralPerShare(maxAge);

b) StableModule::stableCollateralPerShare()
The stableCollateralPerShare() reads the stableBalance to compute _collateralPerShare, which can be zero under the circumstances described in item c). When zero, the _collateralPerShare will also become zero which is used in executeDeposit as above.

   uint256 stableBalance = stableCollateralTotalAfterSettlement(_maxAge);
    _collateralPerShare = (stableBalance * (10 ** decimals())) / totalSupply;

c) StableModule::stableCollateralTotalAfterSettlement() Stable collateral balance will be 0 if vault's total stable collateral is less then netTotal in leverage module as below. In such case _stableCollateralBalance will be returned as 0.

       int256 netTotal = ILeverageModule(vault.moduleAddress(FlatcoinModuleKeys._LEVERAGE_MODULE_KEY))
            .fundingAdjustedLongPnLTotal({maxAge: _maxAge});
       int256 totalAfterSettlement = int256(vault.stableCollateralTotal()) - netTotal;

       if (totalAfterSettlement < 0) {
           _stableCollateralBalance = 0;

As a result of this flow, in the executeDeposit(), the stableCollateralPerShare will return as 0 resulting in throwing the exception.

Impact

Denial of service due to division by zero error.

Code Snippet

in the executeDeposit() function, refer to line 70 where _liquidityMinted is computed. If stableCollateralPerShare() returns 0, then the problem will occur.

https://github.com/sherlock-audit/2023-12-flatmoney/blob/main/flatcoin-v1/src/StableModule.sol#L61-L70

In stableCollateralPerShare() function, if stableBalance is 0, _collateralPerShare will also result in 0. Refer to line 211 to 214 in the below code reference. https://github.com/sherlock-audit/2023-12-flatmoney/blob/main/flatcoin-v1/src/StableModule.sol#L208-L219

stableCollateralTotalAfterSettlement() function, refer to lines from 190 to 193, when totalAfterSettlement is less than 0, _stableCollateralBalance will also become 0. This number contributes to stableCollateralPerShare() resulting on division by zero error.

https://github.com/sherlock-audit/2023-12-flatmoney/blob/main/flatcoin-v1/src/StableModule.sol#L173-L197

Tool used

Manual Review

Recommendation

Check at the time of deposit for total balance of collateral and leveraged positions. If the circumstances prevent minting of more Unit tokens, notify that elegantly to the user.

Duplicate of #142

sherlock-admin commented 8 months ago

1 comment(s) were left on this issue during the judging contest.

takarez commented:

invalid