Collateral can become insolvent

[code423n4]

Lines of code

https://github.com/prepo-io/prepo-monorepo/blob/feat/2022-12-prepo/apps/smart-contracts/core/contracts/ManagerWithdrawHook.sol#L17
https://github.com/prepo-io/prepo-monorepo/blob/feat/2022-12-prepo/apps/smart-contracts/core/contracts/ManagerWithdrawHook.sol#L29-L33
https://github.com/prepo-io/prepo-monorepo/blob/feat/2022-12-prepo/apps/smart-contracts/core/contracts/Collateral.sol#L82

Vulnerability details

Impact

Collateral can become insolvent because of managerWithdraw function.

Proof of Concept

Collateral.managerWithdraw allows to withdraw base tokens to manager.
https://github.com/prepo-io/prepo-monorepo/blob/feat/2022-12-prepo/apps/smart-contracts/core/contracts/Collateral.sol#L80-L83

  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);
  }

In case if managerWithdrawHook is 0 then there is no any checks and _amount is sent to manager. This can make Collateral insolvent.
Scenario.
1.User deposit 1 million tokens to Collateral.
2.managerWithdraw is called and all tokens were sent to manager.
3.Collateral is insolvent, users can't withdraw.

In case if managerWithdrawHook is not 0 then managerWithdrawHook.hook is called.
https://github.com/prepo-io/prepo-monorepo/blob/feat/2022-12-prepo/apps/smart-contracts/core/contracts/ManagerWithdrawHook.sol#L17

  function hook(address, uint256, uint256 _amountAfterFee) external view override { require(collateral.getReserve() - _amountAfterFee >= getMinReserve(), "reserve would fall below minimum"); }

The check that is done here is that balance of base token of Collateral will stay above some provided percentage(minReservePercentage) multiplied by all amount recorded in depositRecord after withdraw.
https://github.com/prepo-io/prepo-monorepo/blob/feat/2022-12-prepo/apps/smart-contracts/core/contracts/ManagerWithdrawHook.sol#L41

  function getMinReserve() public view override returns (uint256) { return (depositRecord.getGlobalNetDepositAmount() * minReservePercentage) / PERCENT_DENOMINATOR; }

Even if this percentage is small, it's enough to make Collateral insolvent as well and Collateral will not hold enough funds to cover users withdraws.

Tools Used

VsCode

Recommended Mitigation Steps

I believe that manager should not be able to withdraw users funds. He should not be able to withdraw any amount that will make Collateral balance less than depositRecord.getGlobalNetDepositAmount().

[hansfriese]

duplicate of #254

[c4-judge]

Picodes marked the issue as duplicate of #254

[c4-judge]

Picodes marked the issue as satisfactory