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

13 stars 11 forks source link

LRTConfigRoleChecker.updateLRTConfig allows permanent loss of admin privileges and can lead to DoS scenario #497

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/utils/LRTConfigRoleChecker.sol#L47 https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/utils/LRTConfigRoleChecker.sol#L27

Vulnerability details

Impact

The LRTConfigRoleChecker.updateLRTConfig function is designed to allow administrators to replace the protocol configuration. However, there is a flaw in the implementation. The admin role is a part of the configuration itself, and replacing the configuration results in transferring admin privileges to the new admin specified in the new configuration. This change causes the previous admin to lose their privileges.

The issue becomes critical if the admin accidentally inputs an incorrect address for the new configuration (e.g., due to a copy-paste error), and if there is no valid configuration contract deployed at that address. In such a scenario, the system becomes inoperable, lacking proper configuration. Furthermore, this situation is irrecoverable because there is no valid admin to revert the changes.

Recommended Mitigation Steps

Consider using a 2-phase upgrade of the configuration:

  1. The administrator proposes to replace the configuration
  2. The new administrator in the new configuration must accept the change.
  3. The old configuration stays in place if the change is not accepted.

Assessed type

DoS

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

c4-judge commented 10 months ago

fatherGoose1 changed the severity to QA (Quality Assurance)

c4-judge commented 10 months ago

fatherGoose1 marked the issue as grade-b