Kelp DAO | rsETH - anarcheuz's results

Lines of code

https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTDepositPool.sol#L119

Vulnerability details

Impact

According to the logic of the protocol (https://blog.kelpdao.xyz/exploring-a-new-defi-primitive-liquid-restaked-token-lrt-ed0a8f63a4e2), minted tokens can be swapped on AMMs. This is a serious problem as prices on AMMs follow a bonding curve that are independent from the Chainlink pricing feed. This will create arbitrage opportunities for hackers to profit using the price difference to mint the LRT token on LRTDepositPool (that uses chainlink for pricing) and the price to swap on external AMMs. Furthermore, depending on which AMMs, prices in these AMMs could be manipulated with flashloans.

The end result would be the protocol funds being drained.

Proof of Concept

https://blog.kelpdao.xyz/exploring-a-new-defi-primitive-liquid-restaked-token-lrt-ed0a8f63a4e2:

Restakers can swap their LRT tokens for other tokens on AMMs or choose to redeem underlying assets through LRT contracts.

Tools Used

Manual review.

Recommended Mitigation Steps

Do not use external AMMs to redeem as the prices are not the same as those returned by chainlink.

Assessed type

Other

Lines of code

https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTDepositPool.sol#L119

Vulnerability details

Impact

Prices are fetched from Chainlink but it is possible that the prices are stale (due to to the fact that it is not checking for the freshness of the price) or chainlink could be experiencing a crash event where it would return prices between a minimum and maximum pre-defined. In these instances, an attacker could make use of this opportunity to delay a user transaction to force the user to incur high slippage.

Proof of Concept

    function depositAsset(
        address asset,
        uint256 depositAmount
    )
        external
        whenNotPaused
        nonReentrant
        onlySupportedAsset(asset)
    {
        // checks
        if (depositAmount == 0) {
            revert InvalidAmount();
        }
        if (depositAmount > getAssetCurrentLimit(asset)) {
            revert MaximumDepositLimitReached();
        }

        if (!IERC20(asset).transferFrom(msg.sender, address(this), depositAmount)) {
            revert TokenTransferFailed();
        }

        // interactions
        uint256 rsethAmountMinted = _mintRsETH(asset, depositAmount);

        emit AssetDeposit(asset, depositAmount, rsethAmountMinted);
    }

Tools Used

Manual review.

Recommended Mitigation Steps

Add a slippage check.

Assessed type

Other

Lines of code

https://github.com/code-423n4/2023-11-kelp/blob/main/src/NodeDelegator.sol#L86

Vulnerability details

Impact

transferBackToLRTDepositPool() does not check if the LRTDepositPool is not paused before sending tokens to it. If the contract was indefinitely paused due to being compromised, the funds would be forever bricked in the LRTDepositPool.

Proof of Concept

    function transferBackToLRTDepositPool(
        address asset,
        uint256 amount
    )
        external
        whenNotPaused
        nonReentrant
        onlySupportedAsset(asset)
        onlyLRTManager
    {
        address lrtDepositPool = lrtConfig.getContract(LRTConstants.LRT_DEPOSIT_POOL);

        if (!IERC20(asset).transfer(lrtDepositPool, amount)) {
            revert TokenTransferFailed();
        }
    }

Tools Used

Manual review.

Recommended Mitigation Steps

It is recommended to check if LRTDepositPool is not paused before sending tokens to it.

Assessed type

Token-Transfer