Dopex - Nyx's results

Lines of code

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

Vulnerability details

Impact

Due to the weth amount not being updated inside the LP vault, the settle() function may revert.

Proof of Concept

function settle(
    uint256[] memory optionIds
  )
    external
    onlyRole(DEFAULT_ADMIN_ROLE)
    returns (uint256 amountOfWeth, uint256 rdpxAmount)
  {
    _whenNotPaused();
    (amountOfWeth, rdpxAmount) = IPerpetualAtlanticVault(
      addresses.perpetualAtlanticVault
    ).settle(optionIds);
    for (uint256 i = 0; i < optionIds.length; i++) {
      optionsOwned[optionIds[i]] = false;
    }

    reserveAsset[reservesIndex["WETH"]].tokenBalance += amountOfWeth;
    reserveAsset[reservesIndex["RDPX"]].tokenBalance -= rdpxAmount;

    emit LogSettle(optionIds);
  }

When the admin settles the options, the collateral token(weth) is transferred from Lp vault to rdpxV2Core.

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

And the transferred weth amount is decreased from the LP vault's total collateral balance. The problem is, when a premium is bought, the total Collateral balance isn't updated.

POC : rdpxV2-core Unit.t.sol

function testLPVaultWETHBalance() public {
        //@audit First LP Vault has 100 weth balance
        console2.log(
            "VaultLp WETH Balance :",
            weth.balanceOf(address(vaultLp))
        );

        rdpxV2Core.bond(5 * 1e18, 0, address(this));
        rdpxV2Core.bond(1 * 1e18, 0, address(this));

        //@audit After calling the bond() function(buying premium), LP Vaults weth balance didnt increased
        console2.log(
            "VaultLp WETH Balance After Bond() :",
            weth.balanceOf(address(vaultLp))
        );

        vault.addToContractWhitelist(address(rdpxV2Core));
        uint256[] memory _ids = new uint256[](1);

        _ids[0] = 0;
        rdpxPriceOracle.updateRdpxPrice(1e7);
        rdpxV2Core.settle(_ids);

        //@audit After calling the settle() function, LP Vaults weth balance decreased
        console2.log(
            "VaultLp WETH Balance After Settle() :",
            weth.balanceOf(address(vaultLp))
        );

        rdpxV2Core.bond(10 * 1e18, 0, address(this));
        rdpxV2Core.bond(1 * 1e18, 0, address(this));

        //@audit Again LP Vaults balance isnt increased
        console2.log(
            "VaultLp WETH Balance After Second bond() calls :",
            weth.balanceOf(address(vaultLp))
        );

        _ids = new uint256[](3);

        _ids[0] = 1;
        _ids[1] = 2;
        _ids[2] = 3;

        rdpxPriceOracle.updateRdpxPrice(1e6);
        rdpxV2Core.settle(_ids);

        //@audit And here as we can see LP vaults weth balance only decreasing
        console2.log(
            "VaultLp WETH Balance After Second Settle() call :",
            weth.balanceOf(address(vaultLp))
        );
    }

Due to the LP Vaults weth balance isn't tracked correctly, this line may revert when the admin trying to settles the options.

Tools Used

Manual Review

Recommended Mitigation Steps

Update the LP Vaults weth balance when a premium is bought.

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

Due to the wrong totalWethDelegated, bond and bondWithDelegate functions may not be usable.

Proof of Concept

When the user uses the addToDelegate() function, the totalWethDelegated is updated. But If the user changes his mind and wants to withdraw his delegated funds, the totalWethDelegated is not updated.

POC Unit.t.sol

function testTotalWethDelegated() public {
        //@audit
        //
        console2.log("totalWethDelegated :", rdpxV2Core.totalWethDelegated());
        console2.log("WETH Balance :", weth.balanceOf(address(rdpxV2Core)));

        uint256 delegateId = rdpxV2Core.addToDelegate(50 * 1e18, 10 * 1e8);

        console2.log(
            "totalWethDelegated After Delegate :",
            rdpxV2Core.totalWethDelegated()
        );
        console2.log(
            "WETH Balance After Delegate :",
            weth.balanceOf(address(rdpxV2Core))
        );

        rdpxV2Core.withdraw(delegateId);

        console2.log(
            "WETH Balance After Withdraw :",
            weth.balanceOf(address(rdpxV2Core))
        );
        console2.log(
            "totalWethDelegated After Withdraw :",
            rdpxV2Core.totalWethDelegated()
        );
    }

the sync() function may revert because of this line.

// reLP
    if (isReLPActive) IReLP(addresses.reLPContract).reLP(_amount);

And due to that, bond() and bondWithDelegate() functions may not be usable because the sync() function is called inside the reLP() function.

Tools Used

Manual Review

Recommended Mitigation Steps

Update the totalWethDelegated when users withdraw.

Assessed type

DoS

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L810-L933

Vulnerability details

Impact

Users can bond with less rDPX amount than it should be. Also, Users who come in with ETH deposit it in a pool may get less rDPX.

Proof of Concept

When users use bond() or bondWithDelegate() functions, the bond cost or rDPX amount is calculated inside the calculateBondCost() function. To calculate the rdpxRequired, the function gets the rDPX price from rdpxPriceOracle. The problem is, the rDPX price may not be updated. Assuming that the keeper updates the price via the update() function. Due to the price is not updated inside the bond functions, anyone can front-run the keeper's tx and bond more WETH with less rDPX amount.

POC : Integration.t.sol

function testRDPXPrice() public {
        //@audit

        // First rpdx price(.312 eth)
        address[] memory path;
        path = new address[](2);
        path[0] = address(weth);
        path[1] = address(rdpx);
        router.swapExactTokensForTokens(
            500e18,
            0,
            path,
            address(this),
            block.timestamp
        );
        rdpxPriceOracle.updateRdpxPrice(312 * 1e5);
        uint price1 = rdpxV2Core.getRdpxPrice();
        console2.log("Price 1 :", price1);

        (uint256 rdpxReq, uint256 wethReq) = rdpxV2Core.calculateBondCost(
            10 * 1e18,
            0
        );
        console2.log("Required rdpx before price update", rdpxReq);

        //@audit lets say the user frontruns here!
        rdpxV2Core.bond(10 * 1e18, 0, address(1));

        //update price to (0.2 eth)
        path[1] = address(weth);
        path[0] = address(rdpx);
        router.swapExactTokensForTokens(
            2000e18,
            0,
            path,
            address(this),
            block.timestamp
        );
        //@audit As we can see here, if the user frontruns the keeper, the user can bond more weth with less rDPX!
        rdpxPriceOracle.updateRdpxPrice(2 * 1e7);
        uint price2 = rdpxV2Core.getRdpxPrice();
        console2.log("Price 2 :", price2);

        (rdpxReq, wethReq) = rdpxV2Core.calculateBondCost(10 * 1e18, 0);
        console2.log("Required rdpx after price update :", rdpxReq);
    }

Tools Used

Foundry

Recommended Mitigation Steps

To correctly calculate the rDPX amount, the rpdx price needs to be updated inside the bond functions.

Assessed type

Other