prePO contest - ayeslick's results

Lines of code

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

Vulnerability details

Impact

An operator can bypass the withdrawal limits if he withdraws when the lastUserPeriodReset + userPeriodLength or lastGlobalPeriodReset + globalPeriodLength is less than the block.timestamp. This causes a DOS as well since globalAmountWithdrawnThisPeriod will be greater than globalWithdrawLimitPerPeriod which will prevent any one from withdrawing until lastGlobalPeriodReset + globalPeriodLength is less than the block.timestamp.

Proof of Concept

An operator deposits an amount that's greater than globalWithdrawLimitPerPeriod when lastGlobalPeriodReset + globalPeriodLength is just less than the block.timestamp. This allows the operator to deposit more than globalWithdrawLimitPerPeriod It also prevents other customers from withdrawing until lastGlobalPeriodReset + globalPeriodLength is less than the block.timestamp.

Recommended Mitigation Steps

Place the require statement in the else block in the if block for both if statements.

if (lastGlobalPeriodReset + globalPeriodLength < block.timestamp) {
      require(globalAmountWithdrawnThisPeriod + _amountBeforeFee <= globalWithdrawLimitPerPeriod, "global withdraw limit exceeded");
      lastGlobalPeriodReset = block.timestamp;
      globalAmountWithdrawnThisPeriod = _amountBeforeFee;
    } else {
      require(globalAmountWithdrawnThisPeriod + _amountBeforeFee <= globalWithdrawLimitPerPeriod, "global withdraw limit exceeded");
      globalAmountWithdrawnThisPeriod += _amountBeforeFee;
    }