sherlock-audit / 2023-07-perennial-judging

2 stars 1 forks source link

cartlex - COORDINATOR CAN'T UPDATE RISK PARAMETERS AFTER updateCoordinator() FUNCTION USED BY OWNER. #83

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

cartlex

medium

COORDINATOR CAN'T UPDATE RISK PARAMETERS AFTER updateCoordinator() FUNCTION USED BY OWNER.

Summary

Functions with onlyCoordinator modifier in Market.sol contract will always throw after owner updated the coordinator if coordinator is not the owner.

Vulnerability Detail

/// @notice Only the coordinator or the owner can call // @audit if owner changed coordinator function will throw due to coordinator != owner.
    modifier onlyCoordinator {
        if (msg.sender != coordinator && msg.sender != factory().owner()) revert MarketNotCoordinatorError();
        _;
    } 

Impact

The owner of the contract decide to update the coordinator and use updateCoordinator() function. A new coordinator now can't use function with onlyCoordinator modifier due to condition that coordinator != owner.

Code Snippet

https://github.com/sherlock-audit/2023-07-perennial/blob/main/perennial-v2/packages/perennial/contracts/Market.sol#L606-L610

Tool used

Manual Review

Recommendation

Remove msg.sender != factory().owner() condition.

/// @notice Only the coordinator or the owner can call
    modifier onlyCoordinator {
        if (msg.sender != coordinator) revert MarketNotCoordinatorError();
        _;
    }
sherlock-admin commented 1 year ago

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

141345 commented:

d

darkart commented:

Works as designed

panprog commented:

invalid because the check reverts if both conditions are true: for coordinator the first condition will be false and it will proceed without reverting