c4-submissions commented 10 months ago

Lines of code

https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTOracle.sol#L70 https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTConfig.sol#L85 https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTConfig.sol#L86

Vulnerability details

Impact

LRTOracle:getRSETHPrice() which is executed in all protocol pricing operations, includes all assets supported in the calculation. This asset is added with the addNewSupportedAsset() function, However, this sets the asset permanently and can't be undone if the asset returns same values when strategy is no longer handled and the asset is no longer supported on EigenLayer.

Proof of Concept

_addNewSupportedAsset() function sets assets as supported forever:

function _addNewSupportedAsset(address asset, uint256 depositLimit) private {
        UtilLib.checkNonZeroAddress(asset);
        if (isSupportedAsset[asset]) {
            revert AssetAlreadySupported();
        }
        isSupportedAsset[asset] = true; // @audit
        supportedAssetList.push(asset);
        depositLimitByAsset[asset] = depositLimit;
        emit AddedNewSupportedAsset(asset, depositLimit);
    }

There is no function that can undo this.

LRTOracle:getRSETHPrice()

function getRSETHPrice() external view returns (uint256 rsETHPrice) {

        ...

        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); // @audit
            totalETHInPool += totalAssetAmt * assetER;

            unchecked {
                ++asset_idx;
            }
        }

        return totalETHInPool / rsEthSupply;
    }

Tools Used

Manual Review

Recommended Mitigation Steps

Contract should have a removeAsset function

Assessed type

Other

c4-pre-sort commented 10 months ago

raymondfam marked the issue as insufficient quality report

c4-pre-sort commented 10 months ago

raymondfam marked the issue as duplicate of #38

c4-judge commented 10 months ago

fatherGoose1 changed the severity to QA (Quality Assurance)