Omniscia Steer Protocol Audit

WhitelistRegistry Manual Review Findings

WhitelistRegistry Manual Review Findings

WRY-01M: Inexistent Manager Deletion Workflow

TypeSeverityLocation
Logical FaultWhitelistRegistry.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:

contracts/WhitelistRegistry.sol
13/**
14 * @dev add whitelist permissions for any number of addresses.
15 * @param _vaultAddress the vault whose whitelist will be edited
16 * @param _addresses the addresses to be added to the whitelist
17 */
18function addPermissions(
19 address _vaultAddress,
20 address[] calldata _addresses
21) external {
22 // Make sure caller is authorized
23 require(
24 msg.sender == whitelistManagers[_vaultAddress],
25 "Only whitelist manager can call this function"
26 );
27
28 // Add permissions
29 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 contract
38 * @param manager the address of the vault's whitelist manager
39 * 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.