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

Attacker can force the use of chainlinks instead of TWAP oracles #72

Open
c4-bot-9 opened this issue Mar 29, 2024 · 6 comments
Open

Attacker can force the use of chainlinks instead of TWAP oracles #72

c4-bot-9 opened this issue Mar 29, 2024 · 6 comments
Labels
bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue grade-b insufficient quality report This report is not of sufficient quality primary issue Highest quality submission among a set of duplicates Q-16 QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax 🤖_54_group AI based duplicate group recommendation

Comments

@c4-bot-9
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/libraries/LibOracle.sol#L102-L106

Vulnerability details

Impact

When the price of a token changes significantly, if the TWAP is intentionally not used, an attacker can benefit from the price difference.

Proof of Concept

In LibOracle.baseOracleCircuitBreaker, if the cached price and the newly retrieved price from Chainlink are significantly different, the UniswapV3 TWAP oracle is used. If the WETH balance in the Uniswap pool is less than 100 ether, the TWAP is not used, and the price from Chainlink is used instead.

Attackers can manipulate the balance of the Uniswap pool using Uniswap's flashswap feature. In other words, attackers can intentionally disable TWAP.

function baseOracleCircuitBreaker(
    uint256 protocolPrice,
    uint80 roundId,
    int256 chainlinkPrice,
    uint256 timeStamp,
    uint256 chainlinkPriceInEth
) private view returns (uint256 _protocolPrice) {
    ...
    } else if (priceDeviation) {
        // Check valid twap price
@>      try IDiamond(payable(address(this))).estimateWETHInUSDC(C.UNISWAP_WETH_BASE_AMT, 30 minutes) returns (uint256 twapPrice)
        {
            ...
            // Save the price that is closest to saved oracle price
            if (chainlinkDiff <= twapDiff) {
                return chainlinkPriceInEth;
            } else {
                // Check valid twap liquidity
                IERC20 weth = IERC20(C.WETH);
                uint256 wethBal = weth.balanceOf(C.USDC_WETH);
@>              if (wethBal < 100 ether) {
@>                  return chainlinkPriceInEth;
                }
                return twapPriceInEth;
            }
        } catch {
            return chainlinkPriceInEth;
        }
    } else {
        return chainlinkPriceInEth;
    }
}

The Chainlink oracle updates slower than DEX. When the price of a token changes significantly, if the TWAP is intentionally not used, an attacker can benefit from the price difference.

Tools Used

Manual Review

Recommended Mitigation Steps

Check UniswapV3 flashswap in this way.

(, , , , , , bool unlocked) = pool.slot0();
require(unlocked, "UniswapV3 reentrancy");

Assessed type

Oracle

@c4-bot-9 c4-bot-9 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Mar 29, 2024
c4-bot-3 added a commit that referenced this issue Mar 29, 2024
@c4-bot-11 c4-bot-11 added the 🤖_54_group AI based duplicate group recommendation label Apr 5, 2024
@c4-pre-sort
Copy link

raymondfam marked the issue as insufficient quality report

@c4-pre-sort c4-pre-sort added the insufficient quality report This report is not of sufficient quality label Apr 6, 2024
@c4-pre-sort
Copy link

raymondfam marked the issue as primary issue

@c4-pre-sort c4-pre-sort added the primary issue Highest quality submission among a set of duplicates label Apr 6, 2024
@raymondfam
Copy link

raymondfam commented Apr 6, 2024

This is meant more for black swan scenario where the delta is set at 50%. In practice, price diff between order price and oracle > chainlink threshold is capped at 0.5%.

@hansfriese
Copy link

The impact is low after paying a 0.3% flashswap fee. QA is appropriate.

@c4-judge c4-judge added downgraded by judge Judge downgraded the risk level of this issue QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax and removed 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value labels Apr 17, 2024
@c4-judge
Copy link
Contributor

hansfriese changed the severity to QA (Quality Assurance)

@c4-judge
Copy link
Contributor

hansfriese marked the issue as grade-b

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue grade-b insufficient quality report This report is not of sufficient quality primary issue Highest quality submission among a set of duplicates Q-16 QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax 🤖_54_group AI based duplicate group recommendation
Projects
None yet
Development

No branches or pull requests

7 participants