Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[H-02] Wrong decimals precision when using RdpxEthOracle as oracle returns price in 18 decimals #624

Closed
code423n4 opened this issue Sep 1, 2023 · 5 comments
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate-549 edited-by-warden satisfactory satisfies C4 submission criteria; eligible for awards sufficient quality report This report is of sufficient quality upgraded by judge Original issue severity upgraded from QA/Gas by judge

Comments

@code423n4
Copy link
Contributor

code423n4 commented Sep 1, 2023

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/core/RdpxV2Core.sol#L605
https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/core/RdpxV2Core.sol#L669
https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/core/RdpxV2Core.sol#L673
https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/perp-vault/PerpetualAtlanticVault.sol#L27
https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/reLP/ReLPContract.sol#L253
https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/reLP/ReLPContract.sol#L274
https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/reLP/ReLPContract.sol#L255
https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/reLP/ReLPContract.sol#L275
https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/core/RdpxV2Core.sol#L1172
https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/core/RdpxV2Core.sol#L1181
https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/perp-vault/PerpetualAtlanticVaultLP.sol#L281

Vulnerability details

Impact

All prices are assume to return 8 decimal precision, but the oracle clearly return all prices in 18 decimals as seen in RdpxEthOracle.sol. The two price functions used are getLpPriceInETH() and getRdpxPriceInEth().

getRdpxPriceInEth():

RdpxEthOracle.sol#L250

/// @notice Returns the price of rDPX in ETH
/// @return price price of rDPX in ETH in 1e18 decimals
function getRdpxPriceInEth() external view override returns (uint price) {
    require(
        blockTimestampLast + timePeriod + nonUpdateTolerance >
            block.timestamp,
        "RdpxEthOracle: UPDATE_TOLERANCE_EXCEEDED"
    );

    // @audit returns prices in 1e18 decimals as seen by using `amountIn` of 1e18
->  price = consult(token0, 1e18);

    require(price > 0, "RdpxEthOracle: PRICE_ZERO");
}
  • This will cause a heavy overestimation of rdpx amount when converting amount of rDPX to equivalent wETH amount.
  • This will cause a heavy underestimation of rDPX amount when converting wETH to rDPX

getLpPriceInETH():

RdpxEthOracle.sol#L217

/// @dev Returns the price of LP in ETH in 1e18 decimals
function getLpPriceInEth() external view override returns (uint) {
    uint totalSupply = pair.totalSupply();
    (uint r0, uint r1, ) = pair.getReserves();
    uint sqrtK = HomoraMath.sqrt(r0.mul(r1)).fdiv(totalSupply); // in 2**112
    uint px0 = getETHPx(token0); // in 2**112
    uint px1 = 2 ** 112; // in 2**112
    // fair token0 amt: sqrtK * sqrt(px1/px0)
    // fair token1 amt: sqrtK * sqrt(px0/px1)
    // fair lp price = 2 * sqrtK * sqrt(px0 * px1)
    // split into 2 sqrts multiplication to prevent uint overflow (note the 2**112)
    uint lpPriceIn112x112 = sqrtK
        .mul(2)
        .mul(HomoraMath.sqrt(px0))
        .div(2 ** 56)
        .mul(HomoraMath.sqrt(px1))
        .div(2 ** 56);

    //@audit returns price in 18 decimals as noticed by the multiplication of 1e18
->  return (lpPriceIn112x112 * 1e18) >> 112;
}
  • Not used in the codebase other than in view function getLpPrice(), which is in turn used in getLpTokenBalanceInWeth()
  • But if used, will heavily overestimate Lp token balance in terms of wETH

In the RdpxV2Core.sol core contract, it can cause the following:

  • DoS of bonding mechanism (regardless of type) due to _transfer() reverting from underflow here (assuming APP puts are not allowed to be purchased)
  • DoS of upperDepeg() decollaterization, if slippage not specified, since slippage (minOut) will be overestimated here
  • Dos of lowerDepeg recollaterization, as price check here will always revert here

In the PerpetualAtlanticVault.sol contract, it can cause the following:

  • Strike price is calculated with a decimal of 18 precision, leading to required collateral being overestimated here and than the subsequent check fails. This can DoS the purchase of APP options and bonding mechanism as well since initial purchase of APP options is done through bonding.

In the ReLPContract.sol contract, it can cause the following:

  • Assuming _transfer() and purchase() did not revert, the reLP process will revert due to overestimated amount of rDPX by a factor of 10 to be swapped out using half of wETH here. This reverts the whole reLP process and consequently, the bonding process.

In the PerpetualAtlanticVaultLP.sol contract, it can cause the following:

  • Hugely overestimated total collateral value in PerpetualAtlanticVaultLP.convertToShares() here, leading to rounding down of shares to 0 and DoSing option writers from depositing collateral into LP vault due to a check reverting here and affecting APP option process.

Tools Used

Manual Analysis

Recommendation

In the following LOC, change precision of operators, division or multiplication from 1e8 to 1e18

In the function ReLPContract.reLP(), change precision dividing from 1e16 to 1e26

In the function RdpxV2Core.calculateBondCost(), multiply by 1e18 Instead of DEFAULT_PRECISION in the following LOC:

Assessed type

Context

@code423n4 code423n4 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Sep 1, 2023
code423n4 added a commit that referenced this issue Sep 1, 2023
@c4-pre-sort
Copy link

bytes032 marked the issue as duplicate of #549

@c4-pre-sort c4-pre-sort added duplicate-549 sufficient quality report This report is of sufficient quality labels Sep 9, 2023
@c4-pre-sort
Copy link

bytes032 marked the issue as sufficient quality report

@c4-judge
Copy link
Contributor

GalloDaSballo marked the issue as satisfactory

@c4-judge c4-judge added satisfactory satisfies C4 submission criteria; eligible for awards and removed 3 (High Risk) Assets can be stolen/lost/compromised directly labels Oct 20, 2023
@c4-judge
Copy link
Contributor

GalloDaSballo changed the severity to 2 (Med Risk)

@c4-judge c4-judge added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value downgraded by judge Judge downgraded the risk level of this issue and removed 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value labels Oct 20, 2023
@c4-judge
Copy link
Contributor

GalloDaSballo changed the severity to 3 (High Risk)

@c4-judge c4-judge added 3 (High Risk) Assets can be stolen/lost/compromised directly upgraded by judge Original issue severity upgraded from QA/Gas by judge and removed downgraded by judge Judge downgraded the risk level of this issue labels Oct 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate-549 edited-by-warden satisfactory satisfies C4 submission criteria; eligible for awards sufficient quality report This report is of sufficient quality upgraded by judge Original issue severity upgraded from QA/Gas by judge
Projects
None yet
Development

No branches or pull requests

4 participants