search

code-423n4 / 2022-01-sherlock-findings

0 stars 0 forks source link

Non-transferable critical privileged role #242

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

gzeon

Vulnerability details

Impact

DEPLOYER is a constant in Manager and it is the only role that can call setSherlockCoreAddress to change sherlockCore address. Consider this is a critical function and there might be a need to change the deplorer address in the future (e.g. governance upgrade), it would be better to use the inherited Ownable pattern to ensure the privileges can be transferred when required.

Proof of Concept

https://github.com/code-423n4/2022-01-sherlock/blob/c763f10c4b5fe2127677d6c25b83adcf3bcec212/contracts/managers/Manager.sol#L35

jack-the-pug commented 2 years ago

setSherlockCoreAddress can only be called once:

https://github.com/code-423n4/2022-01-sherlock/blob/c763f10c4b5fe2127677d6c25b83adcf3bcec212/contracts/managers/Manager.sol#L37-L37

if (address(sherlockCore) != address(0)) revert InvalidConditions();