Topmark - Blocked Accounts are not Checked before Transfer to New Innocent Users

[sherlock-admin2]

Topmark

medium

Blocked Accounts are not Checked before Transfer to New Innocent Users

Summary

Blocked Account are not Checked before Transfer to Innocent New Users

Vulnerability Detail

function blockAccountVersion(uint256 version) external onlyOwner {
        if (version == 0 || version > latestAccountVersion) revert FactoryErrors.InvalidAccountVersion();
 >>>       accountVersionBlocked[version] = true;

        // unsafe cast: accountVersion <= latestAccountVersion, which is a uint88.
        emit AccountVersionBlocked(uint88(version));
    }

The code above from the factory contract shows the blockAccountVersion(...) function and how it is Implemented, it can be noted from the pointer how Blocked account Versions are set to true, but the problem is that none of this factors is put into consideration during account transfer to a new User as provided below from the same contract, this can be used to take advantage of innocent users who get this block data transfered to them with restrictions that affect flow of code execution

function safeTransferAccount(address to) public {
        if (to == address(0)) revert FactoryErrors.InvalidRecipient();

        uint256 id = accountIndex[msg.sender];
        if (id == 0) revert FactoryErrors.OnlyAccount();

        address from = _ownerOf[id];

        // Underflow of the sender's balance is impossible because we check for
        // ownership above and the recipient's balance can't realistically overflow.
        unchecked {
            _balanceOf[from]--;
            _balanceOf[to]++;
        }

        _ownerOf[id] = to;

        delete getApproved[id];

        if (
            to.code.length != 0
                && ERC721TokenReceiver(to).onERC721Received(msg.sender, from, id, "")
                    != ERC721TokenReceiver.onERC721Received.selector
        ) revert FactoryErrors.UnsafeRecipient();

        emit Transfer(from, to, id);
    }

Impact

Blocked Account are not Checked before Transfer New Innocent Users

Code Snippet

https://github.com/sherlock-audit/2023-12-arcadia/blob/main/accounts-v2/src/Factory.sol#L297 https://github.com/sherlock-audit/2023-12-arcadia/blob/main/accounts-v2/src/Factory.sol#L222

Tool used

Manual Review

Recommendation

Arcadia Protocol should ensure necessary validation dis done to prevent transfer of already blocked account to new users I the Factory Contract

[sherlock-admin2]

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

takarez commented:

invalid

[nevillehuang]

Invalid, there is no correlation between the two functions. blockAccountVersion() is blocking account implementation version from being created as a new account, no relation to transfer of an account.