Missing Duplicate Node Delegator Address Check Could Cause Double Counting of Delegator Balance

[c4-submissions]

Lines of code

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

Vulnerability details

Impact

The addNodeDelegatorContractToQueue() function of the LRTDepositPool contract does not contain a check that the address being added is not already included in the nodeDelegatorQueue. If a duplicate address is added to the nodeDelegatorQueue, then the getAssetDistributionData() function would double count the asset balance from the same delegator address, and return incorrect result. This could cause other consequences, including incorrect getTotalAssetDeposits() return value and incorrect getRSETHPrice(), which affects the amount of rsETH being minted to users.

Proof of Concept

The addNodeDelegatorContractToQueue() function allows the same address to be added more than once

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

If the same address is added more than once, the getAssetDistributionData() function below would return incorrect result due to double counting the asset balance from the same delegator address.

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

Tools Used

Manual Review

Recommended Mitigation Steps

Add a check in the addNodeDelegatorContractToQueue() function that the same address cannot be added if it is already included in the nodeDelegatorQueue

Assessed type

Invalid Validation

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #36

[c4-judge]

fatherGoose1 changed the severity to QA (Quality Assurance)

[c4-judge]

fatherGoose1 marked the issue as grade-b