Missing of _userUpdate in unwrapFor in WJLP.sol

[code423n4]

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.

[kingyetifinance]

Duplicate #209 . This is intended behavior but due to it being implemented not properly in borrowerOperations withdraw collateral, it is a bug still where users will not get their rewards updated in calls from this function.