Manager can steal entire User balance

[code423n4]

Lines of code

https://github.com/prepo-io/prepo-monorepo/blob/feat/2022-12-prepo/apps/smart-contracts/core/contracts/Collateral.sol#L80

Vulnerability details

Impact

It seems MANAGER_WITHDRAW_ROLE can steal entire user balance if SET_MIN_RESERVE_PERCENTAGE_ROLE has set setMinReservePercentage to 0%

Proof of Concept

1. The SET_MIN_RESERVE_PERCENTAGE_ROLE role sets minReservePercentage to 0%

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

2. MANAGER_WITHDRAW_ROLE role simply calls the managerWithdraw with contract balance as _amount

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

3. This calls hook which allow to withdraw everything above reserve

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

4. getMinReserve is implemented as below. Since minReservePercentage is 0 so getMinReserve becomes 0

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

5. This means user will be able to withdraw full collateral.getReserve() - _amountAfterFee which is approx entire balance (without fees)

function getReserve() external view override returns (uint256) { return baseToken.balanceOf(address(this)); }

Recommended Mitigation Steps

Do not allow SET_MIN_RESERVE_PERCENTAGE_ROLE to set setMinReservePercentage to 0

[hansfriese]

duplicate of #254

[c4-judge]

Picodes marked the issue as duplicate of #254

[c4-judge]

Picodes marked the issue as satisfactory