sherlock-audit / 2023-09-Gitcoin-judging

11 stars 7 forks source link

whiteh4t9527 - Old Anchor contracts are still accessible when profile owner switch to a new Anchor via Registry.updateProfileName() #981

Closed sherlock-admin2 closed 11 months ago

sherlock-admin2 commented 11 months ago

whiteh4t9527

medium

Old Anchor contracts are still accessible when profile owner switch to a new Anchor via Registry.updateProfileName()

Registry.updateProfileName() deploys new Anchor contract based on the new _name but fails to deactivate the old Anchor such that the profile owner can still access the old Anchor (e.g., transferring old Anchor's funds).

Vulnerability Detail

Since Anchor.execute() only checks if the msg.sender is the profile owner through the Registry contract, an deactivated Anchor contract can still be execute() even it is not literally bound with a profile anymore.

Impact

Profile owners can access out-of-date Anchor functions

Code Snippet

Registry.sol:

    function updateProfileName(bytes32 _profileId, string memory _name)
        external
        onlyProfileOwner(_profileId)
        returns (address anchor)
    {
        // Generate a new anchor address
        anchor = _generateAnchor(_profileId, _name);

        // Get the profile using the profileId from the mapping
        Profile storage profile = profilesById[_profileId];

        // Set the new name
        profile.name = _name;

        // Remove old anchor
        anchorToProfileId[profile.anchor] = bytes32(0);

        // Set the new anchor
        profile.anchor = anchor;
        anchorToProfileId[anchor] = _profileId;

        // Emit the event that the name was updated with the new data
        emit ProfileNameUpdated(_profileId, _name, anchor);

        // Return the new anchor
        return anchor;
    }

Anchor.sol:

    function execute(address _target, uint256 _value, bytes memory _data) external returns (bytes memory) {
        // Check if the caller is the owner of the profile and revert if not
        if (!registry.isOwnerOfProfile(profileId, msg.sender)) revert UNAUTHORIZED();

        // Check if the target address is the zero address and revert if it is
        if (_target == address(0)) revert CALL_FAILED();

        // Call the target address and return the data
        (bool success, bytes memory data) = _target.call{value: _value}(_data);

        // Check if the call was successful and revert if not
        if (!success) revert CALL_FAILED();

        return data;
    }

Tool used

Manual Review

Recommendation

Anchor.execute() should check if it is the update-to-date Anchor

sherlock-admin commented 11 months ago

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

n33k commented:

invalid, low impact