sherlock-audit / 2024-04-titles-judging

12 stars 9 forks source link

0x73696d616f - `TitlesGraph` is missing an `initialize()` function and is setting the `owner` and the `admin` in the constructor, having no `owner` or `admin` in the proxy #182

Closed sherlock-admin3 closed 6 months ago

sherlock-admin3 commented 6 months ago

0x73696d616f

high

TitlesGraph is missing an initialize() function and is setting the owner and the admin in the constructor, having no owner or admin in the proxy

Summary

TitlesGraph is missing an initialize() function to set the owner and/or the admin, which would leave the proxy without owner or admin.

Vulnerability Detail

owner and admin are set in TitlesGraph::constructor(), in the implementation's storage. Thus, when the implementation is called through the proxy's delegatecall, owner and admin will not be assigned and all the permissioned functions by these roles are not going to work, namely all that use roles and upgradeability functionality.

Impact

Permissioned functions by the owner or admin are not reachable.

Code Snippet

The initialize() function is missing in TitlesGraph, so the code can not be linked. However, it can be seen that the constructor executes the logic that should have been in initialize() instead. TitlesGraph::constructor

constructor(address owner_, address admin_) {
    _initializeOwner(owner_);
    _grantRoles(admin_, ADMIN_ROLE);
}

Tool used

Manual Review

Vscode

Recommendation

Implement the initialize() function:

function initialize(address owner_, address admin_) external initializer {
    _initializeOwner(owner_);
    _grantRoles(admin_, ADMIN_ROLE);
}

Duplicate of #445

0x73696d616f commented 5 months ago

Escalate

This issue is not a duplicate of https://github.com/sherlock-audit/2024-04-titles-judging/issues/272. This one describes how the admin and owner of the proxy will never be set for TitlesGraph. #272 mentions how the implementation of TitlesCore is left uninitialized.

sherlock-admin3 commented 5 months ago

Escalate

This issue is not a duplicate of https://github.com/sherlock-audit/2024-04-titles-judging/issues/272. This one describes how the admin and owner of the proxy will never be set for TitlesGraph. #272 mentions how the implementation of TitlesCore is left uninitialized.

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

WangSecurity commented 5 months ago

Agree with the escalation and believe it should be a duplicate of #445. Planning to accept and duplicate accordingly.

Evert0x commented 5 months ago

Result: Medium Duplicate of #445

sherlock-admin2 commented 5 months ago

Escalations have been resolved successfully!

Escalation status: