Inconsistent Update of _totalCollateral(WETH) value in PerpetualAtlanticVaultLP.sol

[code423n4]

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVaultLP.sol#L159-L174

Vulnerability details

Impact

Whenever shares are redeemed in PerpetualAtlanticVaultLP, there's an accounting deficit that causes a revert in PerpetualAtlanticVaultLP.addProceeds(), leading to a denial of service in multiple functions that makes calls to PerpetualAtlanticVaultLP.addProceeds() through PerpetualAtlanticVault.updateFunding() like; PerpetualAtlanticVault.purchase(), PerpetualAtlanticVault.settle(), PerpetualAtlanticVaultLP.deposit(), and PerpetualAtlanticVaultLP.redeem()

Proof of Concept

1. A user makes deposit to mint shares through PerpetualAtlanticVaultLP.deposit() where funds is transferred from user to the contract and _totalCollatera is updated(increamented). https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVaultLP.sol#L128-L132

 // Need to transfer before minting or ERC777s could reenter.
    collateral.transferFrom(msg.sender, address(this), assets);

    _mint(receiver, shares);

    _totalCollateral += assets;

2. When shares are redeemed, funds(collateral and rpdx) are transferred to user, _rdpxCollateral is updated(decremented) but _totalCollateral is not. https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVaultLP.sol#L162-L175

 _rdpxCollateral -= rdpxAmount;

    beforeWithdraw(assets, shares);

    _burn(owner, shares);

    collateral.transfer(receiver, assets);

    IERC20WithBurn(rdpx).safeTransfer(receiver, rdpxAmount);

    emit Withdraw(msg.sender, receiver, owner, assets, shares);

3. The problem here is, in PerpetualAtlanticVaultLP.addProceeds() a check is done to assert that balanceOf Collateral is equal or greater than _totalCollateral + proceeds, this will NEVER hold if shares have been redeemed. https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVaultLP.sol#L191-L194

require(
      collateral.balanceOf(address(this)) >= _totalCollateral + proceeds,
      "Not enough collateral token was sent"
    );

Instance:

--Alice makes a deposit of 100 collateral token to get equivalent shares, this increases _totalCollateral by 100.
--Alice redeems 50% of her shares to get 50 collateral token transferred back to her by the contract, reducing the contract balance by 50 but no corresponding decrease in _totalCollateral.
--So whenever collateral.balanceOf(address(this)) >= _totalCollateral + proceeds is checked, _totalCollateral will still hold 100+, so the check always fails.
--The ripple effect is that all function that relies on the check fails.

Tools Used

Manual review.

Recommended Mitigation Steps

Update _totalCollateral after transferring collateral to the redeemer.

_totalCollateral -= assets;

Assessed type

Token-Transfer

[c4-pre-sort]

bytes032 marked the issue as duplicate of #867

[c4-judge]

GalloDaSballo marked the issue as satisfactory