Dopex - Hama's results

Lines of code

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

Vulnerability details

Impact

First depositor can deposit a single wei as shares then donate to the vault as the supply gets so big to greatly inflate the ratio so that their returned assets and rdpxAmount values would be minimal. Due to truncation when calling reddem and converting to shares this will be used to steal funds from later depositors.

Proof of Concept

the bug is presented in _convertToAssets

function _convertToAssets(
uint256 shares
) internal view virtual returns (uint256 assets, uint256 rdpxAmount) {
uint256 supply = totalSupply;
return
  (supply == 0)
    ? (shares, 0)
    : (
      shares.mulDivDown(totalCollateral(), supply),
      shares.mulDivDown(_rdpxCollateral, supply)
    );
}

_convertToAssets is called in redeemPreview

function redeemPreview(
uint256 shares
) public view returns (uint256, uint256) {
return _convertToAssets(shares);
}

_convertToAssets is called in redeem

function redeem(
uint256 shares,
address receiver,
address owner
) public returns (uint256 assets, uint256 rdpxAmount) {
..
(assets, rdpxAmount) = redeemPreview(shares);

// Check for rounding error since we round down in previewRedeem.
require(assets != 0, "ZERO_ASSETS");

_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);
}

Tools Used

Manual review

Recommended Mitigation Steps

Either during creation of the vault or for first depositor, lock a small amount of the deposit to avoid this.

Assessed type

Token-Transfer