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

Missing Fallback Oracle #194

Closed
c4-submissions opened this issue Nov 13, 2023 · 6 comments
Closed

Missing Fallback Oracle #194

c4-submissions opened this issue Nov 13, 2023 · 6 comments
Labels
bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue duplicate-723 grade-b QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax sufficient quality report This report is of sufficient quality

Comments

@c4-submissions
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/LRTOracle.sol#L45

Vulnerability details

Missing Fallback Oracle

Impact

Malicious Nodes: As oracles operate on a decentralized network, malicious nodes can manipulate data, causing incorrect execution of smart contracts and misleading outcomes.

Network Outages: Reliance on the internet for data connectivity makes oracles susceptible to network outages, causing delays or failures in contract execution by preventing the oracle from accessing necessary information.
https://medium.com/witnet/fallback-oracles-3112038db0a1

Proof of Concept

    /// @notice Provides Asset/ETH exchange rate
    /// @dev reads from priceFetcher interface which may fetch price from any supported oracle
    /// @param asset the asset for which exchange rate is required
    /// @return assetPrice exchange rate of asset
    function getAssetPrice(address asset) public view onlySupportedAsset(asset) returns (uint256) {
        //@audit-issue Would be wise to have a fallback oracle in placed
        return IPriceFetcher(assetPriceOracle[asset]).getAssetPrice(asset);
    }

    /// @notice Provides RSETH/ETH exchange rate
    /// @dev calculates based on stakedAsset value received from eigen layer
    /// @return rsETHPrice exchange rate of RSETH
    function getRSETHPrice() external view returns (uint256 rsETHPrice) {
        address rsETHTokenAddress = lrtConfig.rsETH();
        uint256 rsEthSupply = IRSETH(rsETHTokenAddress).totalSupply();

        if (rsEthSupply == 0) {
            return 1 ether;
        }

        uint256 totalETHInPool;
        address lrtDepositPoolAddr = lrtConfig.getContract(LRTConstants.LRT_DEPOSIT_POOL);

        address[] memory supportedAssets = lrtConfig.getSupportedAssetList();
        uint256 supportedAssetCount = supportedAssets.length;

        for (uint16 asset_idx; asset_idx < supportedAssetCount;) {
            address asset = supportedAssets[asset_idx];
            uint256 assetER = getAssetPrice(asset);

            uint256 totalAssetAmt = ILRTDepositPool(lrtDepositPoolAddr).getTotalAssetDeposits(asset);
            totalETHInPool += totalAssetAmt * assetER;

            unchecked {
                ++asset_idx;
            }
        }

        return totalETHInPool / rsEthSupply;
    }

Tools Used

Manual Review

Recommended Mitigation Steps

To address these vulnerabilities, the use of fallback oracles is crucial. Fallback oracles act as backup systems, stepping in when the primary oracle fails to ensure reliable data provision. They serve as safeguards against data tampering or unavailability, mitigating the risks of smart contract malfunctions and financial losses.

Assessed type

Oracle

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

raymondfam marked the issue as sufficient quality report

@c4-pre-sort c4-pre-sort added the sufficient quality report This report is of sufficient quality label Nov 16, 2023
@c4-pre-sort
Copy link

raymondfam marked the issue as duplicate of #32

@c4-pre-sort
Copy link

raymondfam marked the issue as not a duplicate

@c4-pre-sort
Copy link

raymondfam marked the issue as primary issue

@c4-pre-sort
Copy link

raymondfam marked the issue as duplicate of #723

@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 3 (High Risk) Assets can be stolen/lost/compromised directly labels Dec 1, 2023
@c4-judge
Copy link
Contributor

c4-judge commented Dec 1, 2023

fatherGoose1 changed the severity to QA (Quality Assurance)

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 duplicate-723 grade-b QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax sufficient quality report This report is of sufficient quality
Projects
None yet
Development

No branches or pull requests

4 participants