prePO contest - imare's results

Lines of code

https://github.com/prepo-io/prepo-monorepo/blob/3541bc704ab185a969f300e96e2f744a572a3640/apps/smart-contracts/core/contracts/WithdrawHook.sol#L59-L72

Vulnerability details

Impact

withdrawHook contract checks that inside a specified length of time only certain amount of withdrawal are possible per user and globally.

But on every period reset the allowed withdraw limit check is missing. And a user can withdraw more that is allowed.

Proof of Concept

https://github.com/prepo-io/prepo-monorepo/blob/3541bc704ab185a969f300e96e2f744a572a3640/apps/smart-contracts/core/contracts/WithdrawHook.sol#L59-L72

Tools Used

Manual review

Recommended Mitigation Steps

Enforce the limit check like this:

    if (lastGlobalPeriodReset + globalPeriodLength < block.timestamp) {
      lastGlobalPeriodReset = block.timestamp;
      globalAmountWithdrawnThisPeriod = _amountBeforeFee;
    } else {
-      require(globalAmountWithdrawnThisPeriod + _amountBeforeFee <= globalWithdrawLimitPerPeriod, "global withdraw limit exceeded");
      globalAmountWithdrawnThisPeriod += _amountBeforeFee;
    }
+
+   require(globalAmountWithdrawnThisPeriod + _amountBeforeFee <= globalWithdrawLimitPerPeriod, "global withdraw limit exceeded");
+    
    if (lastUserPeriodReset + userPeriodLength < block.timestamp) {
      lastUserPeriodReset = block.timestamp;
      userToAmountWithdrawnThisPeriod[_sender] = _amountBeforeFee;
    } else {
-      require(userToAmountWithdrawnThisPeriod[_sender] + _amountBeforeFee <= userWithdrawLimitPerPeriod, "user withdraw limit exceeded");
      userToAmountWithdrawnThisPeriod[_sender] += _amountBeforeFee;
    }
+
+   require(userToAmountWithdrawnThisPeriod[_sender] + _amountBeforeFee <= userWithdrawLimitPerPeriod, "user withdraw limit exceeded");

Lines of code

https://github.com/prepo-io/prepo-monorepo/blob/3541bc704ab185a969f300e96e2f744a572a3640/apps/smart-contracts/core/contracts/TokenSender.sol#L41

Vulnerability details

Impact

TokenSender contract is used to reimburse user fees. But if the balance of PPO token inside the contract is low it can happen that the user don't get any token even if there is some to be sent.

Proof of Concept

If the calculated token amount to send is more that is in the contract balance we simply don't send any

https://github.com/prepo-io/prepo-monorepo/blob/3541bc704ab185a969f300e96e2f744a572a3640/apps/smart-contracts/core/contracts/TokenSender.sol#L41

Tools Used

Manual review

Recommended Mitigation Steps

At least two options

• revert instead of just return; to signal that the TokenSender contract is low on balance. Admin should tip the contract so repeating call will not fail
• return the difference of outputAmount and the token balance. Something like this:

    if (outputAmount == 0) return;
-   if (outputAmount > _outputToken.balanceOf(address(this))) return;
+    if (_outputToken.balanceOf(address(this) == 0) revert("need token refill"); // OR just return 0 but then the problem remains
+   if (outputAmount > _outputToken.balanceOf(address(this)))  {
+       outputAmount -= _outputToken.balanceOf(address(this); //send what you can      
+   }

    _outputToken.transfer(recipient, outputAmount);