Submission Details
Severity:
User can steal reward tokens if the Staking contract uses tokens with different decimals
Summary
The index calculations in update() and updateFor() functions in Staking.sol are valid only for tokens with 18 decimals.
Vulnerability Details
The current implementation of Staking.sol is assuming to use TKN and WETH tokens, but if we need a different token from this it may break the index calculations, and will be able to claim the reward and also to withdraw his deposited tokens.
PoC (Proof-Of-Concept)
/// @notice update the global index of earned rewards
/* @audit
* - Lets TKN and WETH are 18 decimals, we have 10WETH inside the contract
* We will deposit 100 TKN with 18 decimals via deposit function,
* it will call updateFor with msg.sender -> update()
* the totalSupply of TKN will be in 18 decimals the balance of WETH also.
* The _diff variable will be balanceOf WETH in the contract because its first deposit and balance state variable is 0,
* then the _ratio = (18 decimals _diff) * 1e18 / (18 decimals TKN totalSupply) = 0.1 (100,000,000,000,000,000 with 18 decimals).
* Index will be also with 18 decimals and everything is fine.
* Then the body of updateFor continue and __supplied will be 0 so it set supplyIndex for him to current index or 0.1 in this case.
*
* In future actions let's say deposit again with 50TKN
* it will skip update function because the second if will be false cuz WETH amount isn't changed,
* _supplied will be 100e18 from last deposit, _delta will be 0 an function will end.
* What if add more WETH?
* Lets add 15WETH and deposit 50TKN again.
* It the _diff will be 15 (25WETH - 10WETH). _ratio = 15e18 * 1e18 / 200e18 = 0.075 (75,000,000,000,000,000 in decimals).
* Index will be now 0.1 + 0.075 = 0.175 (175,000,000,000,000,000 in decimals) and balance will be updated to 25 * 1e18
* _supplied = 150e18, _supplyIndex = 0.1 (100,000,000,000,000,000), then the supplyIndex for this user will be set to 0.175 (175,000,000,000,000,000 in decimals).
* _ delta = 0.175 (175,000,000,000,000,000 in decimals) - 0.1 (100,000,000,000,000,000) = 0.075 (75,000,000,000,000,000 in decimals).
* _share = 150e18 * 0.075 (75,000,000,000,000,000 in decimals) / 1.18 = 11,250,000,000,000,000,000 (11,25)
* claimable for this user will be set to this.
*
* ----------------
*
* Now let's use TKN with 24 decimals and WETH with 10 decimals
* Again will deposit 100 TKN and will have already 10WETH
* totalSupply will be 100 * 1e24 and _balance will be 10 * 1*10
* _diff will be the balance of WETH, because it's the first deposit.
* then the _ratio = 10 * 1*10 * 1e18 / (100 * 1e24) = 1,000 - how much decimals? confusion
* Index will be set to 1000. supplyIndex[msg.sender] = index (1000 in this case).
* Next deposit with 50TKN, will fail cuz WETH amount isn't changed,
* _supplied will be 100e24 from last deposit, _delta will be 0 an function will end.
* Lets add 15WETH and deposit 50TKN again.
* Now the _diff will be 15 (25WETH - 10WETH with 10 decimals).
* _ratio = 15e10 * 1e18 / 200e24 = 750 (how many decimals 10, 18, 24???).
* index will be updated to 1750.
* _supplied = 150e24, _supplyIndex = 1000, then the supplyIndex for this user will be set to 1750.
* _delta = 1750 - 1000 = 750.
* _share = 150e24 * 750 / 1e18 = 112,500,000,000,
* claimable is set to 112,500,000,000.
* Then let's call claim function
* updateFor(msg.sender) is called first,
* We already know this function so the result is that in the second if end, because WETH amount doesn't change.
* _supplied = 1750. _delta will be 0 and will end.
* Then this contract will transfer 112,500,000,000 WETH to the user which is 11.25 (with current WETH implementation with 18 decimals).
* The balance of WETH will become 250,000,000,000 - 112,500,000,000 = 137,500,000,000
* Then most dangerous if msg.sender call withdraw, the updateFor will call update which will end in the second if, and updateFor will end second if as well,
* and contract will return to him 200e24 TKN and he will be 112,500,000,000 WETH drained from here
*/
function update() public {
uint256 totalSupply = TKN.balanceOf(address(this));
if (totalSupply > 0) {
uint256 _balance = WETH.balanceOf(address(this));
if (_balance > balance) {
uint256 _diff = _balance - balance;
if (_diff > 0) {
uint256 _ratio = _diff * 1e18 / totalSupply;
if (_ratio > 0) {
index = index + _ratio;
balance = _balance;
}
}
}
}
}
/// @notice update the index for a user
/// @param recipient the user to update
function updateFor(address recipient) public {
update();
uint256 _supplied = balances[recipient];
if (_supplied > 0) {
uint256 _supplyIndex = supplyIndex[recipient];
supplyIndex[recipient] = index;
uint256 _delta = index - _supplyIndex;
if (_delta > 0) {
uint256 _share = _supplied * _delta / 1e18;
claimable[recipient] += _share;
}
} else {
supplyIndex[recipient] = index;
}
}
Impact
Critical, because the user can drain almost all reward tokens if make the math right and also to get his staking tokens back, because of functions not following CEI.
Tools Used
Manual
Recommendations
Make constraints to be able to swap tokens with the same decimals or use ERC20.decimals() for precision calculation, also follow CEI in deposit, withdraw and claim functions to not be vulnerable to reentrance, and lastly add a check to not be able to withdraw your deposited tokens after claim reward, or just after claim, set your tokens to 0 as like the reward was set to not be able to withdraw after claim.
Prize pool breakdown
Total prize
20,000 USDC
nSLOC
70628 USDC / LOC
18,000 USDC
2,000 USDC
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.