code-423n4 / 2023-11-kelp-findings

13 stars 11 forks source link

Users may not get rseth due to exchange rate manipulation #727

Closed c4-submissions closed 11 months ago

c4-submissions commented 11 months ago

Lines of code

https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTOracle.sol#L52-L76 https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTDepositPool.sol#L109 https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTDepositPool.sol#L141

Vulnerability details

Impact

In deposit() function _mintRsETH() uses following formula to calculate how much rseth should be minted

rsethAmountToMint = (amount * lrtOracle.getAssetPrice(asset)) / lrtOracle.getRSETHPrice();

The problem is that an attacker can manipulate price of rseth and other users when depositng may get 0 rseth.

Proof of Concept

By minting a small amount of rseth and then transferring a large amount of another asset, the attacker can significantly distort the calculated rseth price. Subsequently, when other users attempt to deposit their assets, they won't receive the correct amount of rseth due to the inflated price.

getTotalAssetDeposits() uses balanceOf() to fetch balances of the contracts

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

Consider not using balanceOf for price calculation

Assessed type

Oracle

c4-pre-sort commented 11 months ago

raymondfam marked the issue as sufficient quality report

c4-pre-sort commented 11 months ago

raymondfam marked the issue as duplicate of #42

c4-judge commented 10 months ago

fatherGoose1 marked the issue as satisfactory