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

13 stars 11 forks source link

Once a strategy is set, it can't be updated, so it won't change to a more efficient strategy. #196

Open c4-submissions opened 11 months ago

c4-submissions commented 11 months ago

Lines of code

https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTConfig.sol#L109-L122

Vulnerability details

Proof of Concept

function updateAssetStrategy(
        address asset,
        address strategy
    )
        external
        onlyRole(DEFAULT_ADMIN_ROLE)
        onlySupportedAsset(asset)
    {
        UtilLib.checkNonZeroAddress(strategy);
        if (assetStrategy[asset] == strategy) {
            revert ValueAlreadyInUse();
        }
        assetStrategy[asset] = strategy;
    }

assetStrategy sets where an asset will be deposited in terms of strategy. In updateAssetStrategy, the value of assetStrategy can be set, but it cannot be updated if it has already been set for an asset.

Currently, Eigenlayer uses a passive hold strategy, and in the future, a strategy with higher interest rates may be introduced.

In such cases, all assets deposited in Kelp will be defaulted to the basic hold strategy, which can diminish the protocol's attractiveness.

Tools Used

VS Code

Recommended Mitigation Steps

Modify updateAssetStrategy to allow updates even for assets that have already been set.

Assessed type

Other

c4-pre-sort commented 11 months ago

raymondfam marked the issue as insufficient quality report

c4-pre-sort commented 11 months ago

raymondfam marked the issue as duplicate of #69

c4-judge commented 10 months ago

fatherGoose1 changed the severity to QA (Quality Assurance)

c4-judge commented 10 months ago

fatherGoose1 marked the issue as grade-b