Yeti Finance contest - UncleGrandpa925's results

Handle

UncleGrandpa925

Vulnerability details

Impact

Users' rewards in Wrapped JLP will be miscalculated & lost. Every interaction with WJLP (wrap, unwrapFor...) will trigger the bug.

Location

Function _userUpdate in WJLP.sol

Explanation of the bug

So the nature of this WJLP is simply a wrap of the JLP, and it itself maintains certain information regarding the rewards that each user is entitled to. This information must be updated whenever there is a change in balances of any users or any users who want to redeem their JOE rewards. The function that was used to do this is _userUpdate

This function has a very similar implementation to how TraderJoe (MasterchefV2) did it, but there is a subtle but significant difference: Whenever TraderJoe updates the pending and rewardDebt information, they always immediately transfer out all pending rewards. However, in YETI's contracts, the rewards is not transferred out immediately but instead accumulated and only transfer when the user calls claimReward.

As such, this leads to a miscalculation of users' rewards, and eventually, users will lose their rewards.

Line 252 of WJLP.sol, the unclaimedJOEReward should be accumulated (+=) instead of just reassigning like TraderJoe, OR the reward should be transferred out immediately.

Proof of Concept

For simplicity, I will ignore the 1e12 division.

Let's consider a new user who has 0 WJLP. Let's denote the action of calling wrap(X, msg.sender, msg.sender, msg.sender) as deposit X

---

At T0 user.amount = 0, accJoePerShare=1, user.rewardDebt = 0

---

---

At T1 (a while after T0): user deposits 100 JLP

• Line 248: accJoePerShare = 2
• Line 252: (skipped) => user.unclaimedJOEReward = 0
• Line 256: user.amount = 0 + 100 = 100
• Line 262: user.rewardDebt = 100 * 2 = 200

---

---

At T2 (a while after T1): user deposits 100 JLP

• Line 248: accJoePerShare = 3
• Line 252: user.unclaimedJOEReward = 100 * 3 - 200 = 100
• Line 256: user.amount = 100 + 100 = 200
• Line 262: user.rewardDebt = 200 * 3 = 600

=> If user claims reward here, he will get 100

---

At T3 (shortly after T2): user deposits 100 JLP

• Line 248: accJoePerShare = 3.01
• Line 252: user.unclaimedJOEReward = 200*3.01-600 = 2
• Line 256: user.amount = 200 + 100 = 300
• Line 262: user.rewardDebt = 300 * 3.01 = 903

=> If the user claims reward here, he will get 2, so he lost 98 units of rewards and it's not recoverable.

---

Recommended Mitigation Steps

To mitigate, either change the formula of line 252 to user.unclaimedJOEReward = unclaimedJOEReward.add(user.amount.mul(accJoePerShare).div(1e12).sub(user.rewardDebt));, or simply do it as JOE, transfer out all the accuredReward at every updates

Handle

UncleGrandpa925

Vulnerability details

Impact

Users' rewards in Wrapped JLP will be miscalculated. Hackers can exploit this to steal users' rewards. All WJLP's unwrapFor transactions will trigger the bug.

Location

Function unwrapFor in WJLP.sol

Explanation of the bug

So the nature of this WJLP is simply a wrap of the JLP, and it itself maintains certain information regarding the rewards that each user is entitled to. This information must be updated whenever there is a change in balances of any users or any users who want to redeem their JOE rewards. The function that was used to do this is _userUpdate

As such, it can be observed that _userUpdate was called in wrap (line 136, right before mint). However, no _userUpdate was called in unwrapFor (line 164 there is nothing). Therefore, whenever users unwrap assets, post unwrapping, they will still receive the rewards as if they have never unwrapped.

As such, this leads to a miscalculation of users' rewards. Also, since the rewardData is not updated when unwrapFor is called, hackers can do a flashloan to pretend to wrap a huge amount of assets, the immediately unwrap it. He then will be entitled to almost all rewards available (Example below).

Proof of Concept

For simplicity, I will ignore the division to 1e12. Let's consider a new user who has 0 WJLP.

---

At T0 user.amount = 0, accJoePerShare=1, user.rewardDebt = 0

---

---

At T1 user wrap(1000000, msg.sender, msg.sender, msg.sender)

• Line 248: accJoePerShare = 2
• Line 252: (skipped) => user.unclaimedJOEReward = 0
• Line 256: user.amount = 0 + 100000 = 100000
• Line 262: user.rewardDebt = 100000 * 2 = 200000

the user immediately unwrapFor(msg.sender,1000000) With the current code, user.unclaimedJOEReward, user.amount, user.rewardDebt won't be changed at all

As such, after the unwrap is completed,

• user.unclaimedJOEReward = 0
• user.amount = 100000
• user.rewardDebt = 200000

Note that at this point, the user still has 0 WJLP

---

At T2 (long after T1) user wrap(1, msg.sender, msg.sender, msg.sender)

• Line 248: accJoePerShare = 20
• Line 252: user.unclaimedJOEReward = 100000 * 20 - 200000 = 1800000 ....

So, user just deposit 1 unit of JLP in and he receives the rewards for whatever total amount he has deposited earlier.

---

Recommended Mitigation Steps

To mitigate, just add the missing _userUpdate to line 164.