Dopex - 0xTiwa's results

Lines of code

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

Vulnerability details

Impact

In the PerpetualAtlanticVaultLP contract, the convertToShares function was to return a given amount of shares received for a given amount of assets but the amounts of shares received has to be converted from a 1e18 precision to a 1e8 precision but instead a 1e8 precision was used directly.

Proof of Concept

function convertToShares(
    uint256 assets
  ) public view returns (uint256 shares) {
    uint256 supply = totalSupply;
    uint256 rdpxPriceInAlphaToken = perpetualAtlanticVault.getUnderlyingPrice();

    uint256 totalVaultCollateral = totalCollateral() +
      ((_rdpxCollateral * rdpxPriceInAlphaToken) / 1e8);
    return
      supply == 0 ? assets : assets.mulDivDown(supply, totalVaultCollateral);
  }

As seen above, the price is gotten from the perpetualAtlanticVault.getUnderlyingPrice()

function getUnderlyingPrice() public view returns (uint256) {
    return IRdpxEthOracle(addresses.assetPriceOracle).getRdpxPriceInEth();
  }

which in turn returns the rdpxEthOracle.getRdpxPriceInEth function, which returns the price of rdpx in eth in 1e18 decimals not 1e8, therefore there is a surplus of 1e10 decimals after each conversion.

Tools Used

Manual review

Recommended Mitigation Steps

The amount of assets converted to shares should be converted in the 1e18 precision not 1e8 precision to avoid surplus 1e10 decimals.

uint256 totalVaultCollateral = totalCollateral() +
      ((_rdpxCollateral * rdpxPriceInAlphaToken) / 1e18);

Assessed type

Decimal

Lines of code

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

Vulnerability details

Impact

The removeAssetFromtokenReserves() is used to remove assets from the reserve tokens and also the reserve assets. It removes the assets details from the reserve assets array and pops the last element of the reserve assets array after overwriting the removed asset but it does not remove the symbol from the reserve token array instead it removes the last symbol without overwriting the removed symbol.

Proof of Concept

function removeAssetFromtokenReserves(
    string memory _assetSymbol
  ) external onlyRole(DEFAULT_ADMIN_ROLE) {
    uint256 index = reservesIndex[_assetSymbol];
    _validate(index != 0, 18);

    // remove the asset from the mapping
    reservesIndex[_assetSymbol] = 0;

    // add new index for the last element
    reservesIndex[reserveTokens[reserveTokens.length - 1]] = index;

    // update the index of reserveAsset with the last element
    reserveAsset[index] = reserveAsset[reserveAsset.length - 1];

    // remove the last element 
    reserveAsset.pop();
    reserveTokens.pop();

    emit LogAssetRemovedFromtokenReserves(_assetSymbol, index);
  }

Tools Used

Manual review.

Recommended Mitigation Steps

Update the removeAssetFromtokenReserves() to be able to pop the right symbol.

function removeAssetFromtokenReserves(
    string memory _assetSymbol
  ) external onlyRole(DEFAULT_ADMIN_ROLE) {
    uint256 index = reservesIndex[_assetSymbol];
    _validate(index != 0, 18);

    // remove the asset from the mapping
    reservesIndex[_assetSymbol] = 0;

    // add new index for the last element
    reservesIndex[reserveTokens[reserveTokens.length - 1]] = index;

    // update the index of reserveAsset with the last element
    reserveAsset[index] = reserveAsset[reserveAsset.length - 1];
    reserveTokens[index] = reserveTokens[reserveTokens.length - 1];

    // remove the last element 
    reserveAsset.pop();
    reserveTokens.pop();

    emit LogAssetRemovedFromtokenReserves(_assetSymbol, index);
  }

Assessed type

Other