Anyone can register wallet allowing anyone to set the iswallet[msg.sender] to true for themselves allowing them to exploit other functions.
Proof of Concept
From the comment on the registerWallet() function below, the registerWallet() function Can only be called by safe deployer or the wallet itself. However, anyone can call the registerWallet() function to set the isWallet state as there is no access control.
/**
* @notice Registers a wallet
* @dev Can only be called by safe deployer or the wallet itself
*/
function registerWallet() external {//@audit anyone can call this function and set isWallet state.
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 of comments and implementation of the function.
Recommended Mitigation Steps
Add access control to the registerWallet() function to ensure that only safe deployer or the wallet itself can call the the function as mentioned in the function comment.
Lines of code
https://github.com/code-423n4/2023-10-brahma/blob/main/contracts/src/core/registries/WalletRegistry.sol#L35-L40
Vulnerability details
Impact
Anyone can register wallet allowing anyone to set the
iswallet[msg.sender]
to true for themselves allowing them to exploit other functions.Proof of Concept
From the comment on the
registerWallet()
function below, theregisterWallet()
functionCan only be called by safe deployer or the wallet itself
. However, anyone can call theregisterWallet()
function to set theisWallet
state as there is no access control.Tools Used
Manual review of comments and implementation of the function.
Recommended Mitigation Steps
Add access control to the
registerWallet()
function to ensure that only safe deployer or the wallet itself can call the the function as mentioned in the function comment.Assessed type
Access Control