Kelp DAO | rsETH - cheatc0d3's results

addNewSupportedAsset function can be more optimized

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

Add more comprehensive checks for the asset address.

function addNewSupportedAsset(address asset, uint256 depositLimit) external onlyRole(LRTConstants.MANAGER) {
    require(asset != address(0), "LRTConfig: Asset is zero address");
    require(asset != address(this), "LRTConfig: Asset cannot be this contract");
    // Additional checks specific to asset properties
    // ...
}

addNewSupportedAsset function can be more optimized

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

Include validation for depositLimit to handle edge cases.

function addNewSupportedAsset(address asset, uint256 depositLimit) external onlyRole(LRTConstants.MANAGER) {
    require(depositLimit > 0, "LRTConfig: Deposit limit must be positive");
    require(depositLimit <= MAX_DEPOSIT_LIMIT, "LRTConfig: Deposit limit exceeds maximum allowed");
    // ...
}

Check Return Values of External Calls

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

Failure to check the return values of external contract calls could lead to unexpected behavior.

function getAssetDistributionData(address asset)
    // ... existing function signature
{
    // ... existing validations
    uint256 ndcsCount = nodeDelegatorQueue.length;
    for (uint256 i = 0; i < ndcsCount; ++i) {
        address ndc = nodeDelegatorQueue[i];
        // ... existing checks

        uint256 ndcBalance = IERC20(asset).balanceOf(ndc);
        require(ndcBalance >= 0, "LRTDepositPool: Balance query failed");

        uint256 stakedBalance = INodeDelegator(ndc).getAssetBalance(asset);
        require(stakedBalance >= 0, "LRTDepositPool: Staked balance query failed");

        assetLyingInNDCs += ndcBalance;
        assetStakedInEigenLayer += stakedBalance;
    }
}

}

transferAssetToNodeDelegator function can be more optimized

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

Missing validation on ndcIndex and amount.

function transferAssetToNodeDelegator(uint256 ndcIndex, address asset, uint256 amount) external nonReentrant onlyLRTManager onlySupportedAsset(asset) {
    require(ndcIndex < nodeDelegatorQueue.length, "LRTDepositPool: Invalid NDC index");
    require(amount > 0, "LRTDepositPool: Transfer amount must be positive");
    address nodeDelegator = nodeDelegatorQueue[ndcIndex];
    // ... existing logic
}

updateMaxNodeDelegatorCount function can be more optimized

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

Missing validation on maxNodeDelegatorCount_.

function updateMaxNodeDelegatorCount(uint256 maxNodeDelegatorCount_) external onlyLRTAdmin {
    require(maxNodeDelegatorCount_ > 0, "LRTDepositPool: Invalid node delegator count");
    maxNodeDelegatorCount = maxNodeDelegatorCount_;
    // ... existing logic
}

maxApproveToEigenStrategyManager function can be more optimized

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

Limited validation of the asset address.

function maxApproveToEigenStrategyManager(address asset)
    external
    override
    onlySupportedAsset(asset)
    onlyLRTManager
{
    require(asset != address(0), "NodeDelegator: Asset is zero address");
    address strategyManager = lrtConfig.getContract(LRTConstants.EIGEN_STRATEGY_MANAGER);
    require(strategyManager != address(0), "NodeDelegator: Strategy Manager address is zero");
    IERC20(asset).approve(strategyManager, type(uint256).max);
}

depositAssetIntoStrategy function can be more optimized

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

The function does not validate the returned strategy address.

function depositAssetIntoStrategy(address asset)
    external
    override
    whenNotPaused
    nonReentrant
    onlySupportedAsset(asset)
    onlyLRTManager
{
    address strategy = lrtConfig.assetStrategy(asset);
    require(strategy != address(0), "NodeDelegator: Strategy address is zero");
    IERC20 token = IERC20(asset);
    // ... remaining logic
}

transferBackToLRTDepositPool function can be more optimized

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

Missing validation on amount.

function transferBackToLRTDepositPool(address asset, uint256 amount)
    external
    whenNotPaused
    nonReentrant
    onlySupportedAsset(asset)
    onlyLRTManager
{
    require(amount > 0, "NodeDelegator: Transfer amount must be positive");
    address lrtDepositPool = lrtConfig.getContract(LRTConstants.LRT_DEPOSIT_POOL);
    require(lrtDepositPool != address(0), "NodeDelegator: LRT Deposit Pool address is zero");
    // ... remaining logic
}

getAssetBalances function can be more optimized

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

Lack of validation for the strategy addresses returned from the eigenlayer strategy manager.

function getAssetBalances()
    external
    view
    override
    returns (address[] memory assets, uint256[] memory assetBalances)
{
    address eigenlayerStrategyManagerAddress = lrtConfig.getContract(LRTConstants.EIGEN_STRATEGY_MANAGER);
    require(eigenlayerStrategyManagerAddress != address(0), "NodeDelegator: Strategy Manager address is zero");

    (IStrategy[] memory strategies,) = IEigenStrategyManager(eigenlayerStrategyManagerAddress).getDeposits(address(this));
    uint256 strategiesLength = strategies.length;

    assets = new address[](strategiesLength);
    assetBalances = new uint256[](strategiesLength);

    for (uint256 i = 0; i < strategiesLength;) {
        address strategyAddress = address(strategies[i]);
        require(strategyAddress != address(0), "NodeDelegator: Strategy address is zero");

        assets[i] = IStrategy(strategyAddress).underlyingToken();
        assetBalances[i] = IStrategy(strategyAddress).userUnderlyingView(address(this));
        unchecked {
            ++i;
        }
    }
}

getAssetBalance function can be more optimized

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

No validation on the returned strategy address.

function getAssetBalance(address asset) external view override returns (uint256) {
    require(asset != address(0), "NodeDelegator: Asset address is zero");

    address strategy = lrtConfig.assetStrategy(asset);
    require(strategy != address(0), "NodeDelegator: Strategy address is zero");

    return IStrategy(strategy).userUnderlyingView(address(this));
}

pause and unpause function can be more optimized

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

These functions are appropriately guarded by role checks, but additional state checks could be implemented.

function pause() external onlyLRTManager {
    require(!paused(), "NodeDelegator: Already paused");
    _pause();
}

function unpause() external onlyLRTAdmin {
    require(paused(), "NodeDelegator: Not paused");
    _unpause();
}

getAssetPrice function can be more optimized

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

Potential lack of validation for the input asset and the corresponding price oracle address.

function getAssetPrice(address asset) public view onlySupportedAsset(asset) returns (uint256) {
    require(asset != address(0), "LRTOracle: Asset is zero address");
    address priceOracle = assetPriceOracle[asset];
    require(priceOracle != address(0), "LRTOracle: Price oracle not set for asset");

    return IPriceFetcher(priceOracle).getAssetPrice(asset);
}

getRSETHPrice function can be more optimized

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

Missing validation on returned values and division by zero.

function getRSETHPrice() external view returns (uint256 rsETHPrice) {
    address rsETHTokenAddress = lrtConfig.rsETH();
    require(rsETHTokenAddress != address(0), "LRTOracle: rsETH address is zero");

    uint256 rsEthSupply = IRSETH(rsETHTokenAddress).totalSupply();
    if (rsEthSupply == 0) {
        return 1 ether;
    }

    uint256 totalETHInPool;
    address lrtDepositPoolAddr = lrtConfig.getContract(LRTConstants.LRT_DEPOSIT_POOL);
    require(lrtDepositPoolAddr != address(0), "LRTOracle: Deposit Pool address is zero");

    // ... remaining logic including checks for asset prices
}

updatePriceOracleFor function can be more optimized

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

Limited validation for input parameters.

function updatePriceOracleFor(address asset, address priceOracle)
    external
    onlyLRTManager
    onlySupportedAsset(asset)
{
    require(asset != address(0), "LRTOracle: Asset is zero address");
    require(priceOracle != address(0), "LRTOracle: Price oracle is zero address");
    assetPriceOracle[asset] = priceOracle;
    emit AssetPriceOracleUpdate(asset, priceOracle);
}

pause function can be more optimized

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

No pre-check to verify if the contract is already paused.

function pause() external onlyLRTManager {
    require(!paused(), "LRTOracle: Contract already paused");
    _pause();
}

unpause function can be more optimized

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

No pre-check to verify if the contract is actually paused before unpausing.

function unpause() external onlyLRTAdmin {
    require(paused(), "LRTOracle: Contract is not paused");
    _unpause();
}

mint function can be more optimized

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

The function might not validate the to address and the amount.

function mint(address to, uint256 amount) external onlyRole(MINTER_ROLE) whenNotPaused {
    require(to != address(0), "RSETH: Mint to zero address");
    require(amount > 0, "RSETH: Mint amount must be positive");
    _mint(to, amount);
}

burnFrom function can be more optimized

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

Potential lack of validation for the account address and the amount.

function burnFrom(address account, uint256 amount) external onlyRole(BURNER_ROLE) whenNotPaused {
    require(account != address(0), "RSETH: Burn from zero address");
    require(amount > 0, "RSETH: Burn amount must be positive");
    _burn(account, amount);
}

updateLRTConfig function can be more optimized

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

Limited validation for the _lrtConfig parameter.

function updateLRTConfig(address _lrtConfig) external override onlyRole(DEFAULT_ADMIN_ROLE) {
    require(_lrtConfig != address(0), "RSETH: LRT Config address is zero");
    lrtConfig = ILRTConfig(_lrtConfig);
    emit UpdatedLRTConfig(_lrtConfig);
}

pause and unpause function can be more optimized

https://github.com/code-423n4/2023-11-kelp/blob/main/src/RSETH.sol#L60 https://github.com/code-423n4/2023-11-kelp/blob/main/src/RSETH.sol#L66

Validate State Before State-Changing Operations:** For functions like pause and unpause, ensure the contract is in the correct state before proceeding.

For pause:

function pause() external onlyLRTManager {
    require(!paused(), "RSETH: Already paused");
    _pause();
}

For unpause:

function unpause() external onlyRole(DEFAULT_ADMIN_ROLE) {
    require(paused(), "RSETH: Not paused");
    _unpause();
}

getAssetPrice function can be more optimized

https://github.com/code-423n4/2023-11-kelp/blob/main/src/oracles/ChainlinkPriceOracle.sol#L37

The function might not validate the existence of a price feed for the given asset.

function getAssetPrice(address asset) external view onlySupportedAsset(asset) returns (uint256) {
    address priceFeedAddress = assetPriceFeed[asset];
    require(priceFeedAddress != address(0), "ChainlinkPriceOracle: Price feed not set for asset");
    return AggregatorInterface(priceFeedAddress).latestAnswer();
}

updateLRTConfig function can be more optimized

https://github.com/code-423n4/2023-11-kelp/blob/main/src/utils/LRTConfigRoleChecker.sol#L47

Limited validation for the lrtConfigAddr parameter.

function updateLRTConfig(address lrtConfigAddr) external virtual onlyLRTAdmin {
    require(lrtConfigAddr != address(0), "LRTConfigRoleChecker: LRT Config address is zero");
    lrtConfig = ILRTConfig(lrtConfigAddr);
    emit UpdatedLRTConfig(lrtConfigAddr);
}

onlyLRTManager modifier can be more optimized

https://github.com/code-423n4/2023-11-kelp/blob/main/src/utils/LRTConfigRoleChecker.sol#L20

The modifier may not validate if lrtConfig address is properly set.

modifier onlyLRTManager() {
    require(address(lrtConfig) != address(0), "LRTConfigRoleChecker: LRT Config not set");
    require(IAccessControl(address(lrtConfig)).hasRole(LRTConstants.MANAGER, msg.sender), "ILRTConfig: Caller not LRT Config Manager");
    _;
}

onlyLRTAdmin modifier can be more optimized

https://github.com/code-423n4/2023-11-kelp/blob/main/src/utils/LRTConfigRoleChecker.sol#L27

Similar to onlyLRTManager, it might not validate if lrtConfig address is properly set.

modifier onlyLRTAdmin() {
    bytes32 DEFAULT_ADMIN_ROLE = 0x00;
    require(address(lrtConfig) != address(0), "LRTConfigRoleChecker: LRT Config not set");
    require(IAccessControl(address(lrtConfig)).hasRole(DEFAULT_ADMIN_ROLE, msg.sender), "ILRTConfig: Caller not LRT Config Admin");
    _;
}

onlySupportedAsset modifier can be more optimized

https://github.com/code-423n4/2023-11-kelp/blob/main/src/utils/LRTConfigRoleChecker.sol#L35

The modifier does not validate the input asset address.

modifier onlySupportedAsset(address asset) {
    require(asset != address(0), "LRTConfigRoleChecker: Asset address is zero");
    require(lrtConfig.isSupportedAsset(asset), "ILRTConfig: Asset not supported");
    _;
}