code-423n4 / 2023-10-brahma-findings

8 stars 7 forks source link

Anyone can call Register a wallet using the function RegisterWallet(). #408

Closed c4-submissions closed 1 year ago

c4-submissions commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-10-brahma/blob/dd0b41031b199a0aa214e50758943712f9f574a0/contracts/src/core/registries/WalletRegistry.sol#L35

Vulnerability details

Impact

Anyone(other smartcontracts) can directly call RegisterWallet() to register wallets.

Proof of Concept

In WalletRegistry.sol the function RegisterWallet(), is used to register wallets for SubAccounts and consoleAccounts but this function is supposed to be only callable by the SafeDeployer.sol and wallets. But this is not ensuerd in the code.

* @notice Registers a wallet
     * @dev Can only be called by safe deployer or the wallet itself
     */
    function registerWallet() external {
        if (isWallet[msg.sender]) revert AlreadyRegistered();
        if (subAccountToWallet[msg.sender] != address(0)) revert IsSubAccount();
        isWallet[msg.sender] = true;
        emit RegisterWallet(msg.sender);
    }

Tools Used

Manual review

Recommended Mitigation Steps

Add function that checks if the caller is a wallet(EOA) if the msg.sender is not SafeDeployer.sol

function isContract(address account) internal view returns (bool) {
    uint256 size;
    assembly {
        size := extcodesize(account)
    }
    return size > 0;
}

This function if added to the contract can be used to run an if/else statement that ensures that the caller is either a wallet or the SafeDeployer.sol.

Assessed type

Access Control

c4-pre-sort commented 1 year ago

raymondfam marked the issue as low quality report

c4-pre-sort commented 1 year ago

raymondfam marked the issue as duplicate of #17

c4-judge commented 1 year ago

alex-ppg marked the issue as unsatisfactory: Invalid