Omniscia Steer Protocol Audit
WhitelistRegistry Manual Review Findings
WhitelistRegistry Manual Review Findings
WRY-01M: Inexistent Manager Deletion Workflow
Type | Severity | Location |
---|---|---|
Logical Fault | WhitelistRegistry.sol:L41-L44 |
Description:
The registerWhitelistManager
permits a manager to be overridden, however, the permissions it has previously bestowed will not be revoked.
Impact:
It is impossible to properly transition to a new whitelist manager in a timely fashion in the current contract design if the previous manager acted maliciously.
Example:
13/**14 * @dev add whitelist permissions for any number of addresses.15 * @param _vaultAddress the vault whose whitelist will be edited16 * @param _addresses the addresses to be added to the whitelist17 */18function addPermissions(19 address _vaultAddress,20 address[] calldata _addresses21) external {22 // Make sure caller is authorized23 require(24 msg.sender == whitelistManagers[_vaultAddress],25 "Only whitelist manager can call this function"26 );27
28 // Add permissions29 uint256 addressCount = _addresses.length;30 for (uint256 i; i != addressCount; ++i) {31 permissions[_vaultAddress][_addresses[i]] = 1;32 }33 emit PermissionsAdded(msg.sender, _vaultAddress, _addresses);34}35
36/**37 * @dev function meant to be called by contracts (usually in initializer) to register a whitelist manager for that contract38 * @param manager the address of the vault's whitelist manager39 * No access control, since any given contract can only modify their own data here.40 */41function registerWhitelistManager(address manager) external {42 whitelistManagers[msg.sender] = manager;43 emit ManagerAdded(msg.sender, manager);44}
Recommendation:
We advise the whitelist system to be revised to detect whether the manager was changed and revoke any previously allocated permissions. To achieve this, the permissions
mapping could use a contract-wide incremental counter that is increased each time the registerWhitelistManager
function is invoked, essentially revoking all previously set permissions on each replacement / assignment.
Alleviation (200f275c40cbd4798f4a416c044ea726755d4741):
After evaluating this exhibit, the Steer Protocol team has opted to retain the current behaviour in place as the smart contract is meant to be integrated with by other contracts in their system and external users who use it should do so while being aware of its caveats. As a result, we consider this exhibit nullified.