Collateral.managerWithdraw: owner can rug all baseToken

[code423n4]

Lines of code

https://github.com/prepo-io/prepo-monorepo//blob/3541bc704ab185a969f300e96e2f744a572a3640/apps/smart-contracts/core/contracts/Collateral.sol#L80-L83

Vulnerability details

Impact

MANAGER_WITHDRAW_ROLE can call Collateral.managerWithdraw to send baseToken to manager

  function managerWithdraw(uint256 _amount) external override onlyRole(MANAGER_WITHDRAW_ROLE) nonReentrant {
    if (address(managerWithdrawHook) != address(0)) managerWithdrawHook.hook(msg.sender, _amount, _amount);
    baseToken.transfer(manager, _amount);
  }

setManager is called by SET_MANAGER_ROLE

  function setManager(address _newManager) external override onlyRole(SET_MANAGER_ROLE) {
    manager = _newManager;
    emit ManagerChange(_newManager);
  }

The number of baseTokens that MANAGER_WITHDRAW_ROLE can take out is determined by ManagerWithdrawHook.minReservePercentage

  function hook(address, uint256, uint256 _amountAfterFee) external view override { require(collateral.getReserve() - _amountAfterFee >= getMinReserve(), "reserve would fall below minimum"); }
...
  function getMinReserve() public view override returns (uint256) { return (depositRecord.getGlobalNetDepositAmount() * minReservePercentage) / PERCENT_DENOMINATOR; }

and setMinReservePercentage is called by SET_MIN_RESERVE_PERCENTAGE_ROLE

  function setMinReservePercentage(uint256 _newMinReservePercentage) external override onlyRole(SET_MIN_RESERVE_PERCENTAGE_ROLE) {
    require(_newMinReservePercentage <= PERCENT_DENOMINATOR, ">100%");
    minReservePercentage = _newMinReservePercentage;
    emit MinReservePercentageChange(_newMinReservePercentage);
  }

These roles can be directly granted by the owner, resulting in the owner being able to rug all baseToken in Collateral

Proof of Concept

https://github.com/prepo-io/prepo-monorepo//blob/3541bc704ab185a969f300e96e2f744a572a3640/apps/smart-contracts/core/contracts/Collateral.sol#L80-L83
https://github.com/prepo-io/prepo-monorepo//blob/3541bc704ab185a969f300e96e2f744a572a3640/apps/smart-contracts/core/contracts/ManagerWithdrawHook.sol#L17-L18

Tools Used

None

Recommended Mitigation Steps

Consider using timelock

[hansfriese]

duplicate of #254

[c4-judge]

Picodes marked the issue as duplicate of #254

[c4-judge]

Picodes marked the issue as satisfactory