eeyore - Lack of access control in the `MarketFactory.updateExtension()` function.

[sherlock-admin2]

eeyore

High

Lack of access control in the MarketFactory.updateExtension() function.

Summary

An attacker can set himself as an extension, which is an allowed protocol-wide operator. As such, he can act on an account's behalf in all its positions and, for example, withdraw its collateral.

Vulnerability Detail

A new authorization functionality was introduced in Perennial 2.3 update to allow for signers and extensions to act on behalf of the account. Unfortunately, the updateExtension() function within the MarketFactory is missing the onlyOwner access control modifier.

File: MarketFactory.sol
100:@>   function updateExtension(address extension, bool newEnabled) external {
101:         extensions[extension] = newEnabled;
102:         emit ExtensionUpdated(extension, newEnabled);
103:     }

This extensions mapping is later used in the authorization() function to determine if the sender is an account operator:

File: MarketFactory.sol
77:     function authorization(
78:         address account,
79:         address sender,
80:         address signer,
81:         address orderReferrer
82:     ) external view returns (bool isOperator, bool isSigner, UFixed6 orderReferralFee) {
83:         return (
84:@>           account == sender || extensions[sender] || operators[account][sender],
85:             account == signer || signers[account][signer],
86:             referralFees(orderReferrer)
87:         );
88:     }

The authorization() function is used within the Market contract to authorize the order in the name of the account:

File: Market.sol
500:         // load factory metadata
501:         (updateContext.operator, updateContext.signer, updateContext.orderReferralFee) =
502:@>           IMarketFactory(address(factory())).authorization(context.account, msg.sender, signer, orderReferrer);
503:         if (guaranteeReferrer != address(0)) updateContext.guaranteeReferralFee = guaranteeReferralFee;
504:     }

File: InvariantLib.sol
78:         if (
79:             !updateContext.signer &&                                            // sender is relaying the account's signed intention
80:@>           !updateContext.operator &&                                          // sender is operator approved for account
81:             !(newOrder.isEmpty() && newOrder.collateral.gte(Fixed6Lib.ZERO))    // sender is depositing zero or more into account, without position change
82:         ) revert IMarket.MarketOperatorNotAllowedError();

As can be seen, anyone without authorization can set himself as an extension and act as the operator of any account, leading to the loss of all funds.

Impact

• Loss of funds.
• Missing access control.

Code Snippet

https://github.com/sherlock-audit/2024-08-perennial-v2-update-3/blob/main/perennial-v2/packages/perennial/contracts/MarketFactory.sol#L100-L103 https://github.com/sherlock-audit/2024-08-perennial-v2-update-3/blob/main/perennial-v2/packages/perennial/contracts/MarketFactory.sol#L77-L88 https://github.com/sherlock-audit/2024-08-perennial-v2-update-3/blob/main/perennial-v2/packages/perennial/contracts/Market.sol#L500-L504 https://github.com/sherlock-audit/2024-08-perennial-v2-update-3/blob/main/perennial-v2/packages/perennial/contracts/libs/InvariantLib.sol#L78-L82

Tool used

Manual Review

Recommendation

Add the onlyOwner modifier to the MarketFactory.updateExtension() function.

[sherlock-admin2]

The protocol team fixed this issue in the following PRs/commits: https://github.com/equilibria-xyz/perennial-v2/pull/443