alexzoid - `Anchor` Address Reuse Across Different `Registry` Versions

[sherlock-admin2]

alexzoid

medium

Anchor Address Reuse Across Different Registry Versions

The function _generateAnchor() in the Registry contract has a potential flaw where, under specific conditions, it may reuse an Anchor contract address from a previous Registry deployment rather than creating a new one. Registry address could be updated with updateRegistry() in Allo.sol. The primary reason is that the salt used for the CREATE3 deployment is solely based on the profileId and the profile name. If a new Registry contract is deployed and tries to recreate an existing profile, it will end up with the same Anchor address from the previous Registry.

Vulnerability Detail

1. The _generateAnchor() function uses CREATE3 to generate or fetch a pre-calculated address for the Anchor contract.
2. The salt for the CREATE3 deployment is created by hashing _profileId and _name.
3. If a profile with the same profileId and _name is created in a new version of the Registry, the salt will remain the same, resulting in the same pre-calculated address.
4. The function checks if a contract already exists at that address. If it does, it reuses that address without checking if the existing contract references the current Registry.

Impact

Profiles created in a newer version of the Registry might reference Anchor contracts tied to a previous version of the Registry. This means the Anchor contract could be checking for ownership in an outdated Registry, potentially leading to unauthorized access or operational inconsistencies.

Proof of Concept

1. Deploy the Registry contract, create a profile, and note the associated Anchor address.
2. Deploy a new version of the Registry contract.
3. In the new Registry, create a profile with the same profileId and _name as the profile from step 1.
4. The new profile will be associated with the Anchor from the old Registry (and could not be changed) rather than having a new Anchor.

Code Snippet

• https://github.com/sherlock-audit/2023-09-Gitcoin/blob/main/allo-v2/contracts/core/Allo.sol#L213-L215
• https://github.com/sherlock-audit/2023-09-Gitcoin/blob/main/allo-v2/contracts/core/Registry.sol#L142
• https://github.com/sherlock-audit/2023-09-Gitcoin/blob/main/allo-v2/contracts/core/Registry.sol#L335-L352
• https://github.com/sherlock-audit/2023-09-Gitcoin/blob/main/allo-v2/contracts/core/Anchor.sol#L72

Tool used

Manual review

Recommendation

It is crucial to include a unique identifier for each version of the Registry in the salt used for CREATE3. This will ensure that even if profiles have the same profileId and _name, their Anchor addresses will be unique across different Registry versions.

Modified _generateAnchor() function:

function _generateAnchor(bytes32 _profileId, string memory _name) internal returns (address anchor) {
    // Incorporate the current registry address into the salt to ensure uniqueness across versions
    bytes32 salt = keccak256(abi.encodePacked(_profileId, _name, address(this)));

    address preCalculatedAddress = CREATE3.getDeployed(salt);

    // check if the contract already exists and if the profileId matches
    if (preCalculatedAddress.code.length > 0) {
        if (Anchor(payable(preCalculatedAddress)).profileId() != _profileId) revert ANCHOR_ERROR();

        anchor = preCalculatedAddress;
    } else {
        bytes memory creationCode = abi.encodePacked(type(Anchor).creationCode, abi.encode(_profileId));

        // Use CREATE3 to deploy the anchor contract
        anchor = CREATE3.deploy(salt, creationCode, 0);
    }
}

[sherlock-admin]

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

n33k commented:

invalid, it would be a bug in the newer version of the Registry