Dopex - grearlake's results

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/core/RdpxV2Core.sol#L764-#L783 https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/perp-vault/PerpetualAtlanticVault.sol#L315-#L369 https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/perp-vault/PerpetualAtlanticVaultLP.sol#L200-#L203

Vulnerability details

Impact

Attacker can DoS settle() function's execution, lead to loss of fund

Proof of Concept

When admin call RDPXV2Core#settle() , it will make a call to PerpetualAtlanticVault#settle():

    (amountOfWeth, rdpxAmount) = IPerpetualAtlanticVault(
        addresses.perpetualAtlanticVault
    ).settle(optionIds);

In PerpetualAtlanticVault#settle(), there is a call to PerpetualAtlanticVaultLP#subtractLoss():

    IPerpetualAtlanticVaultLP(addresses.perpetualAtlanticVaultLP)
        .subtractLoss(ethAmount);

Problem raised in require condition in PerpetualAtlanticVaultLP#subtractLoss():

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

In PerpetualAtlanticVaultLP, _totalCollateral does not accurately reflect amount of collateral in contract. Attacker simply just need to directly deposit any token to PerpetualAtlanticVaultLP but not in deposit() function, this condition will be break, which make function being reverted.

Tools Used

Manual review.

Recommended Mitigation Steps

It is really hard to fix this issue, other mechanism should be implemented to mitigrate this problem

Assessed type

DoS

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/core/RdpxV2Core.sol#L975-#L990

Vulnerability details

Impact

balance of token will be counted wrong because of wrong counting totalWethDelegated in contract

Proof of Concept

We have this scenario: 1, User A addToDelegate() with amount = 10, totalWethDelegated = 10 2, User B call bondWithDelegate() with delegateId belong to user A, amount = 2, totalWethDelegated = 8 3, User A withdraw rest of non-active collateral, totalWethDelegated = 8

Root cause is in withdraw() function, totalWethDelegated value is not updated when user withdraw, which lead to wrong counting in sync() function:

function sync() external {
    for (uint256 i = 1; i < reserveAsset.length; i++) {
        uint256 balance = IERC20WithBurn(reserveAsset[i].tokenAddress)
            .balanceOf(address(this));

        if (weth == reserveAsset[i].tokenAddress) {
            balance = balance - totalWethDelegated;
        }
        reserveAsset[i].tokenBalance = balance;
    }

    emit LogSync();
}

Tools Used

Manual review

Recommended Mitigation Steps

Minus totalWethDelegated when withdraw weth

Assessed type

Other