sherlock-audit / 2023-09-Gitcoin-judging

11 stars 7 forks source link

aycozynfada - Admin can assign manager roles to invalid addresses #951

Closed sherlock-admin2 closed 11 months ago

sherlock-admin2 commented 11 months ago

aycozynfada

high

Admin can assign manager roles to invalid addresses

Admin can mistakenly assign manager roles to an extruder who is not a member of a profile in the -addPoolManager

Vulnerability Detail

During role assignment by the admin using the 'FunctionaddPoolManager'. isOwnerOrMemberOfProfile function" from the registry contract is not used to crosscheck if new manager is valid before assigning manager roles to them. Thereby allowing extruder addresses to gain managerial roles and manipulate contract by causing denial of service attacks to recipients and causing managerial errors detrimental to normal functioning of the contract. https://github.com/sherlock-audit/2023-09-Gitcoin/blob/main/allo-v2/contracts/core/Allo.sol#L263-L269

Impact

High

Code Snippet

` function addPoolManager( uint256 _poolId, address _manager ) external onlyPoolAdmin(_poolId) { // Reverts if the address is the zero address with 'ZERO_ADDRESS()' if (_manager == address(0)) revert ZERO_ADDRESS();

    // Grants the pool manager role to the '_manager' address
    _grantRole(pools[_poolId].managerRole, _manager);
}

`

Tool used

Manual Review

Recommendation

the isOwnerOrMemberOfProfile function from the registry contract should be utilized during pool manager assignment to ascertain if new manager address is a valid addresses.