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

13 stars 11 forks source link

The addNodeDelegatorContractToQueue function lacks a check for nodeDelegatorContracts[i]. #269

Closed c4-submissions closed 11 months ago

c4-submissions commented 11 months ago

Lines of code

https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/LRTDepositPool.sol#L162-L176

Vulnerability details

Impact

After verifying that the address of nodeDelegatorContracts[i] is non-zero, it is necessary to check whether this address has already been added to nodeDelegatorQueue to prevent duplicate additions that could lead to issues.

Proof of Concept

https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/LRTDepositPool.sol#L162-L176 function addNodeDelegatorContractToQueue(address[] calldata nodeDelegatorContracts) external onlyLRTAdmin { uint256 length = nodeDelegatorContracts.length; if (nodeDelegatorQueue.length + length > maxNodeDelegatorCount) { revert MaximumNodeDelegatorCountReached(); }

    for (uint256 i; i < length;) {
        UtilLib.checkNonZeroAddress(nodeDelegatorContracts[i]);

@ nodeDelegatorQueue.push(nodeDelegatorContracts[i]); emit NodeDelegatorAddedinQueue(nodeDelegatorContracts[i]); unchecked { ++i; } } }

Tools Used

vs

Recommended Mitigation Steps

add check

Assessed type

Context

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 #36

c4-judge commented 11 months ago

fatherGoose1 changed the severity to QA (Quality Assurance)

c4-judge commented 11 months ago

fatherGoose1 marked the issue as grade-b