XDZIBEC - XO-The `onlyCollateral` modifier does not check to see if the `controller().collateral()` function returns a valid address.

[sherlock-admin]

XDZIBEC

medium

XO-The onlyCollateral modifier does not check to see if the controller().collateral() function returns a valid address.

Summary

• The onlyCollateral modifier does not check to see if the controller().collateral() function returns a valid address. This means that an attacker can call the onlyCollateral modifier with a contract address that is not the collateral contract.

  Vulnerability Detail

  modifier onlyCollateral {
      if (msg.sender != address(controller().collateral())) revert NotCollateralError();
  
      _;
  }

• There is a vulnerability in the controller() function, the problem is in the the onlyCollateral modifier, it does not check to see if the controller().collateral() function returns a valid address. This means that an attacker could call the onlyCollateral modifier with a contract address that is not the collateral contract, and the modifier would not do anything. This could allow an attacker to bypass the modifier and call the function that is protected by the modifier.

  Impact

• An attacker can bypass the modifier and call the function that is protected by the modifier and this lead to steal funds from the contract.

  Code Snippet

• https://github.com/sherlock-audit/2023-05-perennial/blob/main/perennial-mono/packages/perennial/contracts/controller/UControllerProvider.sol#L53C2-L58C1

  Tool used

Manual Review

Recommendation

• Add the check require(controller().collateral() != address(0)); to the onlyCollateral modifier.