Dopex - 0xsurena's results

Lines of code

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

Vulnerability details

Impact

The call to the settle function can get reverted if a malicious user send some collateral tokens to the PerpetualAtlanticVaultLP.sol contract.

Proof of Concept

At the end of settle process, PerpetualAtlanticVault.sol needs to subtract loss value from _totalCollateral in the PerpetualAtlanticVaultLP.sol. the subtractLoss in the PerpetualAtlanticVaultLP.sol is :

function subtractLoss(uint256 loss) public onlyPerpVault {
require(
  collateral.balanceOf(address(this)) == _totalCollateral - loss,
  "Not enough collateral was sent out"
);
_totalCollateral -= loss;
}

Now if a malicious user send some collateral tokens directly to the PerpetualAtlanticVaultLP.sol contract, settle process can get fail.

1. PerpetualAtlanticVaultLP. _totalCollateral is 15.
2. a malicious user send 5 collateral tokens directly to the PerpetualAtlanticVaultLP.sol contract. balanceOf(address(this)) is now 20 .
3. RDPXV2CORE_ROLE role, send transaction to settle 5 collateral tokens. https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVault.sol#L315
4. 5 collateral token get transfer from perpetual vault to rdpx rdpxV2Core. balanceOf(address(this)) is now 15. https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVault.sol#L347
5. the subtractLoss gets call inside PerpetualAtlanticVaultLP.sol. https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVault.sol#L359

Now in require check, balanceOf(address(this)) is 15 and should be equal to the _totalCollateral - loss or 15 - 5 = 10. but it's not and cause that the settle get reverted.

Same thing is happening with addProceeds function.

function addProceeds(uint256 proceeds) public onlyPerpVault {
    require(
        collateral.balanceOf(address(this)) >= _totalCollateral + proceeds,
        "Not enough collateral token was sent"
    );
    _totalCollateral += proceeds;
}

Tools Used

Manually

Recommended Mitigation Steps

I suggest to remove below check :

require(
  collateral.balanceOf(address(this)) == _totalCollateral - loss,
  "Not enough collateral was sent out"
);

Assessed type

Invalid Validation