xiaoming90 - Ability to revoke access does not expire

[sherlock-admin4]

xiaoming90

High

Ability to revoke access does not expire

Summary

It was observed that PartyA can revoke access as long as the revoke cooldown has passed and the ability to revoke access does not expire. As the ability to revoke access does not expire, this gives back the users the freedom to revoke access anytime, resulting in similar issues described in the documentation where PartyB cannot execute certain actions when it expects to do so.

Root Cause

• The ability to revoke access does not expire

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

It was observed that PartyA can revoke access as long as the revoke cooldown has passed and the ability to revoke access does not expire. Thus, PartyA could simply execute the proposeToRevokeAccesses function when a new account is created in anticipation of the possibility of access revocation in the future. After the revoke cooldown has passed, this gives PartyA the ability to instantly revoke the access whenever they wish since this ability does not expire.

https://github.com/sherlock-audit/2024-09-symmio-v0-8-4-update-contest/blob/main/protocol-core/contracts/multiAccount/MultiAccount.sol#L100

File: MultiAccount.sol
100:    function proposeToRevokeAccesses(address account, address target, bytes4[] memory selector) external onlyOwner(account, msg.sender) {
101:        require(target != msg.sender && target != account, "MultiAccount: Invalid target");
102:        for (uint256 i = selector.length; i != 0; i--) {
103:            revokeProposalTimestamp[account][target][selector[i - 1]] = block.timestamp;
104:        }
105:        emit ProposeToRevokeAccesses(account, target, selector);
106:    }
..SNIP..
114:    function revokeAccesses(address account, address target, bytes4[] memory selector) external onlyOwner(account, msg.sender) {
115:        require(target != msg.sender && target != account, "MultiAccount: Invalid target");
116:        for (uint256 i = selector.length; i != 0; i--) {
117:            require(
118:                revokeProposalTimestamp[account][target][selector[i - 1]] != 0,
119:                "MultiAccount: Revoke access not proposed"
120:            );
121:            require(
122:                revokeProposalTimestamp[account][target][selector[i - 1]] + revokeCooldown <= block.timestamp,
123:                "MultiAccount: Cooldown not reached"
124:            );
125:            delegatedAccesses[account][target][selector[i - 1]] = false;
126:            revokeProposalTimestamp[account][target][selector[i - 1]] = 0;
127:        }
128:        emit DelegateAccesses(account, target, selector, false);
129:    }

Impact

Extracted from the Symm IO v0.8.3 documentation:

freedom in revoking access created challenges for hedgers. For instance, hedgers might assume they could sendQuote on behalf of users during the instantOpen process, only to discover their access had been revoked after they had already hedged the position.

As the ability to revoke access does not expire, this gives back the users the freedom to revoke access anytime, This resulted in similar issues described in the documentation where PartyB cannot execute certain actions when it expects to do so, breaking core protocol functionality.

PoC

No response

Mitigation

The ability to revoke access should expire if PartyA does not trigger it within a certain timeframe.