Kelp DAO | rsETH - AerialRaider's results

Lines of code

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

Vulnerability details

Impact

These input validation enhancements in the transferAssetToNodeDelegator function are designed to safeguard against potential errors and misuse. By ensuring that inputs are within expected bounds and meaningful (non-zero), the contract becomes more robust, secure, and user-friendly.

Proof of Concept

In the transferAssetToNodeDelegator function of the LRTDepositPool contract, the suggested enhancements involve adding input validation checks. These are intended to ensure the function handles only valid and expected data, enhancing its security and reliability. Suggested Changes: Validation for ndcIndex: Added a check to ensure the ndcIndex (Node Delegator Contract Index) is within the bounds of the nodeDelegatorQueue array.

require(ndcIndex < nodeDelegatorQueue.length, "InvalidIndex: NDC index out of bounds");

Validation for amount: Added a check to ensure the amount being transferred is greater than 0.

require(amount > 0, "InvalidAmount: Transfer amount must be greater than 0");

here is the modified function:

function transferAssetToNodeDelegator(uint256 ndcIndex, address asset, uint256 amount) external { require(ndcIndex < nodeDelegatorQueue.length, "InvalidIndex: NDC index out of bounds"); require(amount > 0, "InvalidAmount: Transfer amount must be greater than 0");

// Existing code...

}

Tools Used

VS code

Recommended Mitigation Steps

Rationale Behind the Changes: Index Bounds Check: Ensuring that the ndcIndex is within the bounds of the nodeDelegatorQueue array prevents out-of-bounds access, which is a common source of errors in programming. Accessing an array with an invalid index could lead to unexpected behavior or even cause the contract to revert unexpectedly. This check adds a layer of protection against both inadvertent mistakes and potential malicious attempts to disrupt the contract's functionality. Non-Zero Amount Check: The check for a non-zero transfer amount is crucial to prevent wasteful transactions that don't actually transfer any assets. Transferring zero assets could still consume gas and clutter the blockchain with meaningless transactions. It also helps in maintaining logical consistency within the contract, ensuring that every call to this function has a tangible effect.

Assessed type

Access Control