sherlock-audit / 2023-12-flatmoney-judging

11 stars 9 forks source link

vvv - DoS attack by overflowing marginDepositedTotal value in Vault #169

Closed sherlock-admin closed 6 months ago

sherlock-admin commented 7 months ago

vvv

high

DoS attack by overflowing marginDepositedTotal value in Vault

Summary

marginDepositedTotal could be overflowed due to incorrect check in FlatcoinVault. Overlowed marginDepositedTotal leads to DoS, as it's used in InvariantChecks.

Vulnerability Detail

In settleFundingFees method in FlatcoinVault due to incorrect check for negative marginDepositedTotal, it can be overflowed to extreme values.

  _globalPositions.marginDepositedTotal = (int256(_globalPositions.marginDepositedTotal) > _fundingFees)
            ? uint256(int256(_globalPositions.marginDepositedTotal) + _fundingFees)
            : 0;

Suppose _fundingFees happens to be -1000, and marginDepositedTotal == 900, as we directly cast int256(_globalPositions.marginDepositedTotal) + _fundingFees to unit256 we get type(unit256).max() - 100 in marginDepositedTotal in our example.

Overlowed marginDepositedTotal leads to revert in _getCollateralNet, since solidity 0.8 will revert overflowed addition of unit256s. Reverts in _getCollateralNet leads to DoS for various places in the protocol ([1] [2])

Impact

DoS for the big part of the protocol.

Code Snippet

  _globalPositions.marginDepositedTotal = (int256(_globalPositions.marginDepositedTotal) > _fundingFees)
            ? uint256(int256(_globalPositions.marginDepositedTotal) + _fundingFees)
            : 0;

Tool used

Manual Review

Recommendation

Change check for negative marginDepositedTotal to:

  _globalPositions.marginDepositedTotal = (int256(_globalPositions.marginDepositedTotal) > -_fundingFees)
            ? uint256(int256(_globalPositions.marginDepositedTotal) + _fundingFees)
            : 0;

Duplicate of #195

sherlock-admin commented 6 months ago

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

takarez commented:

valid; high(2)