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

13 stars 11 forks source link

Centralization Risk when it comeds to DEFAULT_ADMIN_ROLE #126

Closed c4-submissions closed 10 months ago

c4-submissions commented 11 months ago

Lines of code

https://github.com/code-423n4/2023-11-kelp/blob/c5fdc2e62c5e1d78769f44d6e34a6fb9e40c00f0/src/LRTDepositPool.sol#L212-L215

Vulnerability details

Impact

The DEFAULT_ADMIN_ROLE is as well the onlyLRTadmin which is in charge of unpausing the LRTDeposit as well as updateMaxNodeDelegatorCount. The DEFAULT_ADMIN_ROLE role has the ability of adding and removing other roles.

The project has not stated whether the DEFAULT_ADMIN_ROLE will be a Timelock Contract which would make sense in this case.

I would advise to either state clearly if this role would be given to the Timelock Contract or add another unpauser role.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

Tools Used

Recommended Mitigation Steps

Assessed type

Access Control

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 primary issue

raymondfam commented 11 months ago

Pertaining to Analysis Report.

c4-judge commented 10 months ago

fatherGoose1 marked the issue as unsatisfactory: Invalid