c4-submissions commented 10 months ago

Lines of code

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

Vulnerability details

Impact

registerWallet in WalletRegistry missing access control

Proof of Concept

the wallet can be registered by calling registerWallet


    /**
     * @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);
    }

as the comment suggest, the method should only be able to called by safe deployer or wallet itself

however, the check that if msg.sender is safe deployer is missing

anyone can register as wallet then call sensitive function such as updatePolicy from PolicyRegistry.sol

Tools Used

Manual Review

Recommended Mitigation Steps

add validation

 if (msg.sender != AddressProviderService._getAuthorizedAddress(_SAFE_DEPLOYER_HASH)) revert InvalidSender();

Assessed type

Access Control

c4-pre-sort commented 10 months ago

raymondfam marked the issue as low quality report

c4-pre-sort commented 10 months ago

raymondfam marked the issue as duplicate of #17

c4-judge commented 10 months ago

alex-ppg marked the issue as unsatisfactory: Invalid

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

registerWallet in WalletRegistry missing access control #450