Platform: Code4rena
Start Date: 09/12/2022
Pot Size: $36,500 USDC
Total HM: 9
Participants: 69
Period: 3 days
Judge: Picodes
Total Solo HM: 2
Id: 190
League: ETH
Rank: 27/69
Findings: 2
Award: $238.90
🌟 Selected for report: 0
🚀 Solo Findings: 0
210.7761 USDC - $210.78
One potential vulnerability in the code is that the userToAmountWithdrawnThisPeriod variable for a sender is not properly checked at the start of each new user period. As a result, it is possible for the value of this variable to become larger than the userWithdrawLimitPerPeriod and for more collateral tokens to be withdrawn than the limit allows.
During withdraw, hook function checks whether the withdrawal is less than userWithdrawLimitPerPeriod. At the start of new user period, that check is missing and withdrawal amount can be more than userWithdrawLimitPerPeriod.
Manual review
Put following check at the start of each new global period.
require(_amountBeforeFee <= userWithdrawLimitPerPeriod, "user withdraw limit exceeded");
#0 - hansfriese
2022-12-13T15:26:22Z
duplicate of #310
#1 - c4-judge
2022-12-13T19:07:23Z
Picodes marked the issue as duplicate of #310
#2 - c4-judge
2023-01-01T17:19:40Z
Picodes marked the issue as satisfactory
🌟 Selected for report: 0xSmartContract
Also found by: 0Kage, 0x52, 0xAgro, 0xNazgul, 0xTraub, 0xhacksmithh, Awesome, Aymen0909, Bnke0x0, Englave, Janio, Mukund, Parth, RaymondFam, Rolezn, SmartSek, Tointer, UdarTeam, Udsen, Zarf, caventa, chaduke, csanuragjain, deliriusz, gz627, idkwhatimdoing, izhelyazkov, joestakey, neumo, obront, oyc_109, rvierdiiev, shark, trustindistrust, wait, yongskiws
28.124 USDC - $28.12
https://github.com/prepo-io/prepo-monorepo/blob/feat/2022-12-prepo/apps/smart-contracts/core/contracts/Collateral.sol#L80-L83 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#L41
hook function in ManagerWithdrawHook is used for ensuring that the amount remaining after withdrawn by manager is above certain threshold(returned by getMinReserve). However, manager can take advantage of this when seeing large withdrawal by user and frontrunning transaction by withdrawing more tokens. If manager would have withdrawn after the withdraw by user, it would receive less tokens.
assume minReservePercentage = 50%. There are 100 tokens in the reserve. And some user sends the transaction to withdraw 20 tokens. After withdrawal by user, manager will only be able to withdraw (100 - 20) * 0.5 = 40 tokens but if manager withdraws first, then it will be able to withdraw (100 * 0.5) = 50 tokens and after withdrawal by user, the reserve will fall below minimum reserve.
Manual review
Use timelock for withdrawal by manager so that it can't frontrun other users.
#0 - Picodes
2022-12-19T11:07:55Z
Low severity as the impact isn't significant compared to just withdrawing.
#1 - c4-judge
2022-12-19T11:08:01Z
Picodes changed the severity to QA (Quality Assurance)
#2 - c4-judge
2022-12-19T14:26:58Z
Picodes marked the issue as grade-b
#3 - ramenforbreakfast
2022-12-21T23:13:17Z
This is fine, reserve is intended to prevent manager from withdrawing beyond the reserve, it is not necessarily a undesirable situation that a user withdraws when Collateral is below reserve requirement.
#4 - c4-sponsor
2022-12-21T23:13:22Z
ramenforbreakfast marked the issue as sponsor disputed