sherlock-audit / 2023-09-Gitcoin-judging

11 stars 7 forks source link

Tri-pathi - Anchor deployement can be always failed #964

Closed sherlock-admin closed 11 months ago

sherlock-admin commented 11 months ago

Tri-pathi

medium

Anchor deployement can be always failed

Anchor deployement can be always failed

Vulnerability Detail

constructor of Anchor is

File: contracts/core/Anchor.sol
 constructor(bytes32 _profileId) {
        registry = Registry(msg.sender);
        profileId = _profileId;
    }

https://github.com/allo-protocol/allo-v2/blob/0b881ef4a0013d2809374c9ea69f4cf1288dfe62/contracts/core/Anchor.sol#L55C4-L58C6 it creates a new instance of Registry which needs to be initialize

File: contracts/core/Registry.sol
    function initialize(address _owner) external reinitializer(1) {
        // Make sure the owner is not 'address(0)'
        if (_owner == address(0)) revert ZERO_ADDRESS();

        // Grant the role to the owner
        _grantRole(ALLO_OWNER, _owner);
    }

https://github.com/allo-protocol/allo-v2/blob/0b881ef4a0013d2809374c9ea69f4cf1288dfe62/contracts/core/Registry.sol#L79

At new instance Attacker can call this function just after the deployment of Anchor and set owner role to any address. which will make overall deployment fail.

This can lead to DOS as every time Attacker will make deployement fail

Impact

DOS

Code Snippet

https://github.com/allo-protocol/allo-v2/blob/0b881ef4a0013d2809374c9ea69f4cf1288dfe62/contracts/core/Anchor.sol#L55C4-L58C6

Tool used

Manual Review

Recommendation

A best way to mitigate this issue will be deploying strategy(making a new instance) and initialize in same transaction so Attacker won't able to call initialize the function.

sherlock-admin commented 11 months ago

1 comment(s) were left on this issue during the judging contest.

n33k commented:

invalid, Registry creates Anchor, not the reverse