Zellic - Audit Reports

Category: Business Logic

Operator change prevents revocation of prior authorizations

Medium Impact

Medium Severity

Medium Likelihood

Description

When the factory updates its operator address, account owners lose the ability to revoke authorizations previously granted to the old operator. Both revokeOperatorBorrowing and approveAuthorizationRevocation fetch the current operator address from the factory when constructing revocation calls:

function revokeOperatorBorrowing(address service) external onlyOwner {
    [...]
    address operatorAddress = IManagementAccountFactory(_factory).operator();
    IService.Call[] memory calls = IService(service).buildRevokeAuthorization(
        _creditAuthorizationType, operatorAddress, settlementTokenAddress
    );
    [...]
}

function approveAuthorizationRevocation() external onlyOperator nonReentrant {
    [...]
    address operatorAddress = IManagementAccountFactory(_factory).operator();
    IService.Call[] memory calls = IService(service).buildRevokeAuthorization(
        _creditAuthorizationType, operatorAddress, settlementTokenAddress
    );
    [...]
}

As a result, these functions attempt to revoke permissions for the new operator (who was never authorized) instead of the previously authorized operator, leaving the old operator's permissions intact on lending protocols.

Impact

Authorizations granted to previous operators cannot be revoked and remain active on lending protocols indefinitely. The old operator retains the ability to borrow against the account's collateral on any service where they were previously authorized.

Revocation attempts execute without reverting, providing no indication to users that the wrong operator was targeted or that the revocation failed.

Recommendations

Store the authorized operator address when authorization is granted, and then use that stored address during revocation instead of fetching from the factory:

+mapping(address => address) private _authorizedOperators;

function authorizeOperatorBorrowing(
    address service,
    AuthorizationType authType,
    uint256 allowance
) external onlyOwner nonReentrant {
    [...]
    address operatorAddress = IManagementAccountFactory(_factory).operator();
+   _authorizedOperators[service] = operatorAddress;
    [...]
}

function revokeOperatorBorrowing(address service) external onlyOwner {
    [...]
-   address operatorAddress = IManagementAccountFactory(_factory).operator();
+   address operatorAddress = _authorizedOperators[service];
+   if (operatorAddress == address(0)) {
+       revert ManagementAccountErrors.NoAuthorizationFound(service);
+   }
    IService.Call[] memory calls = IService(service).buildRevokeAuthorization(
        _creditAuthorizationType, operatorAddress, settlementTokenAddress
    );
    [...]
+   delete _authorizedOperators[service];
}

function approveAuthorizationRevocation() external onlyOperator nonReentrant {
    [...]
+   address service = _pendingAuthRevocationService;
-   address operatorAddress = IManagementAccountFactory(_factory).operator();
+   address operatorAddress = _authorizedOperators[service];
+   if (operatorAddress == address(0)) {
+       revert ManagementAccountErrors.NoAuthorizationFound(service);
+   }
    [...]
+   delete _authorizedOperators[service];
}

Remediation

This issue has been acknowledged by Hyperbeat, and a fix was implemented in commit 582d51b5↗.