sherlock-audit / 2023-10-perennial-judging

11 stars 7 forks source link

kaancaglan - Centralization risk for privileged functions|19| #16

Closed sherlock-admin closed 10 months ago

sherlock-admin commented 10 months ago

kaancaglan

medium

Centralization risk for privileged functions|19|

Summary

The contracts contain several functions with privileged access, which are only accessible by the owner or admin. These functions are central to the contract's functionality but pose a centralization risk if the privileged roles are not managed securely.

Vulnerability Detail

The owner-only functions include critical operations like updating beneficiaries, coordinators, and market parameters, which are susceptible to misuse if the owner's account is compromised. This centralization introduces a single point of failure and trust dependency.

Impact

If the owner account is compromised, malicious actors could alter the contract's behavior, leading to loss of funds or manipulation of the contract's intended functionality. The centralization risk also affects the contract's decentralization ethos.

Code Snippet

[M-01] Centralization risk for privileged functions

Severity

Contracts with privileged functions need owner/admin to be trusted not to perform malicious updates or drain funds. This may also cause a single point failure.

Number Of Instances Found

19

Code Location

Click to show findings ```solidity Path: ./perennial-v2/packages/perennial/contracts/Market.sol 96: function updateBeneficiary(address newBeneficiary) external onlyOwner { // @audit-issue 103: function updateCoordinator(address newCoordinator) external onlyOwner { // @audit-issue 110: function updateParameter(MarketParameter memory newParameter) external onlyOwner { // @audit-issue 124: function updateReward(Token18 newReward) public onlyOwner { // @audit-issue ``` *GitHub*: [96](https://github.com/sherlock-audit/2023-10-perennial/blob/main/./perennial-v2/packages/perennial/contracts/Market.sol#L96-L96), [103](https://github.com/sherlock-audit/2023-10-perennial/blob/main/./perennial-v2/packages/perennial/contracts/Market.sol#L103-L103), [110](https://github.com/sherlock-audit/2023-10-perennial/blob/main/./perennial-v2/packages/perennial/contracts/Market.sol#L110-L110), [124](https://github.com/sherlock-audit/2023-10-perennial/blob/main/./perennial-v2/packages/perennial/contracts/Market.sol#L124-L124) ```solidity Path: ./perennial-v2/packages/perennial-vault/contracts/Vault.sol 147: function register(IMarket market) external onlyOwner { // @audit-issue 174: function updateMarket(uint256 marketId, uint256 newWeight, UFixed6 newLeverage) external onlyOwner { // @audit-issue 195: function updateParameter(VaultParameter memory newParameter) external onlyOwner { // @audit-issue 209: function claimReward() external onlyOwner { // @audit-issue ``` *GitHub*: [147](https://github.com/sherlock-audit/2023-10-perennial/blob/main/./perennial-v2/packages/perennial-vault/contracts/Vault.sol#L147-L147), [174](https://github.com/sherlock-audit/2023-10-perennial/blob/main/./perennial-v2/packages/perennial-vault/contracts/Vault.sol#L174-L174), [195](https://github.com/sherlock-audit/2023-10-perennial/blob/main/./perennial-v2/packages/perennial-vault/contracts/Vault.sol#L195-L195), [209](https://github.com/sherlock-audit/2023-10-perennial/blob/main/./perennial-v2/packages/perennial-vault/contracts/Vault.sol#L209-L209) ```solidity Path: ./perennial-v2/packages/perennial-oracle/contracts/OracleFactory.sol 48: function register(IOracleProviderFactory factory) external onlyOwner { // @audit-issue 55: function authorize(IFactory caller) external onlyOwner { // @audit-issue 64: function create(bytes32 id, IOracleProviderFactory factory) external onlyOwner returns (IOracle newOracle) { // @audit-issue 80: function update(bytes32 id, IOracleProviderFactory factory) external onlyOwner { // @audit-issue 92: function updateMaxClaim(UFixed6 newMaxClaim) external onlyOwner { // @audit-issue ``` *GitHub*: [48](https://github.com/sherlock-audit/2023-10-perennial/blob/main/./perennial-v2/packages/perennial-oracle/contracts/OracleFactory.sol#L48-L48), [55](https://github.com/sherlock-audit/2023-10-perennial/blob/main/./perennial-v2/packages/perennial-oracle/contracts/OracleFactory.sol#L55-L55), [64](https://github.com/sherlock-audit/2023-10-perennial/blob/main/./perennial-v2/packages/perennial-oracle/contracts/OracleFactory.sol#L64-L64), [80](https://github.com/sherlock-audit/2023-10-perennial/blob/main/./perennial-v2/packages/perennial-oracle/contracts/OracleFactory.sol#L80-L80), [92](https://github.com/sherlock-audit/2023-10-perennial/blob/main/./perennial-v2/packages/perennial-oracle/contracts/OracleFactory.sol#L92-L92) ```solidity Path: ./perennial-v2/packages/perennial-oracle/contracts/keeper/KeeperFactory.sol 111: function authorize(IFactory factory) external onlyOwner { // @audit-issue 119: function associate(bytes32 id, bytes32 underlyingId) external onlyOwner { // @audit-issue 133: function create(bytes32 id) public virtual onlyOwner returns (IKeeperOracle newOracle) { // @audit-issue 240: function updateGranularity(uint256 newGranularity) external onlyOwner { // @audit-issue ``` *GitHub*: [111](https://github.com/sherlock-audit/2023-10-perennial/blob/main/./perennial-v2/packages/perennial-oracle/contracts/keeper/KeeperFactory.sol#L111-L111), [119](https://github.com/sherlock-audit/2023-10-perennial/blob/main/./perennial-v2/packages/perennial-oracle/contracts/keeper/KeeperFactory.sol#L119-L119), [133](https://github.com/sherlock-audit/2023-10-perennial/blob/main/./perennial-v2/packages/perennial-oracle/contracts/keeper/KeeperFactory.sol#L133-L133), [240](https://github.com/sherlock-audit/2023-10-perennial/blob/main/./perennial-v2/packages/perennial-oracle/contracts/keeper/KeeperFactory.sol#L240-L240) ```solidity Path: ./perennial-v2/packages/perennial-reward/contracts/Reward.sol 38: function register(IFactory factory) external onlyOwner { // @audit-issue 46: function mint(address to, uint256 amount) external onlyOwner { // @audit-issue ``` *GitHub*: [38](https://github.com/sherlock-audit/2023-10-perennial/blob/main/./perennial-v2/packages/perennial-reward/contracts/Reward.sol#L38-L38), [46](https://github.com/sherlock-audit/2023-10-perennial/blob/main/./perennial-v2/packages/perennial-reward/contracts/Reward.sol#L46-L46)

Tool used

Manual Review - Used my own static analyzer (still in development)

Recommendation

It's recommended to implement multi-signature or decentralized governance mechanisms for privileged functions to mitigate the risk of centralization. Alternatively, consider using time-locks for critical operations to allow for community review and intervention if necessary. Regular security audits and restricting access to owner-only functions can also help to minimize risks.

sherlock-admin2 commented 10 months ago

3 comment(s) were left on this issue during the judging contest.

panprog commented:

invalid, centralization issues are invalid by Sherlock rules

tsvetanovv commented:

Information

chainNue commented:

watson need to read https://docs.sherlock.xyz/audits/judging/judging