Skip to content

Latest commit

 

History

History
62 lines (44 loc) · 3.36 KB

105.md

File metadata and controls

62 lines (44 loc) · 3.36 KB

Fast Khaki Raccoon

High

Users can abuse a reentrancy lock to disallow the share value from going up

Summary

Users can abuse a reentrancy lock to disallow the share value from going up

Root Cause

Users can abuse a reentrancy lock upon DecentralizedIndex::burn() to disallow the share value from going up:

function burn(uint256 _amount) external lock { ... }

The _burnRewards() function has the following code:

try IPEAS(rewardsToken).burn(_burnAmount) {} catch {
            IERC20(rewardsToken).safeTransfer(address(0xdead), _burnAmount);
}

If we call burn() on a locked pod (pOHM is one of the most recommended paired LP tokens in the website which is a pod), we would revert, go in the catch and transfer to the dead address. However, burning and transferring have different state changes - one is a simple transfer while the other one decreases the total supply. In this case, we would simply transfer and the total supply won't go down which means the share value won't go down as expected.

Internal Pre-conditions

No response

External Pre-conditions

No response

Attack Path

  1. A malicious user, Alice, targets a TokenRewards contract which has an underlying pod where the PAIRED_LP_TOKEN is a reward token (an expected scenario handled by the code), the PAIRED_LP_TOKEN is pOHM which is a pod, this is one of the recommended paired LP tokens in the Peapods front-end upon creating a pod
  2. Alice calls DecentralizedIndex.flashMint() (pOHM) minting herself 0 tokens to avoid paying actual fees and has her malicious contract as the recipient, this locks the contract due to the lock modifier (function flashMint(...) external override lock)
  3. She reenters into TokenRewards.claimReward() using her malicious contract which then calls DecentralizedIndex.processPreSwapFeesAndSwap() (this is not a pOHM index this time but if it was, the same issue will happen without the reentrancy part. This means that the lock modifier there won't revert, we locked the pOHM pod)
  4. The checks in _processPreSwapFeesAndSwap() pass (note that _shortCircuitRewards is not 1 in this contract, it's 1 in the pOHM pod) and we go to _feeSwap() where the PAIRED_LP_TOKEN is the reward token and we call TokenRewards.depositRewardsNoTransfer() with the paired LP token which is the pOHM pod
  5. We get to TokenRewards._depositRewards() where we call _burnRewards() when the token provided is a reward token (our scenario)
  6. We reach this code:
try IPEAS(rewardsToken).burn(_burnAmount) {} catch {
            IERC20(rewardsToken).safeTransfer(address(0xdead), _burnAmount);
}
  1. We fail to burn due to the locked pOHM pod and we simply transfer to the dead address, these result in different states as explained

Impact

Share value will not go down as expected, that is the reason for burning. Also, the last user to withdraw will still have to pay fees as we have this check there to confirm whether he is the last one out:

return _debondAmount >= (_totalSupply * 99) / 100;

As the total supply can not go to 0 as the shares are in the dead address, even the last user will have to pay fees.

PoC

No response

Mitigation

Fix is non-entirely trivial, consider changing the pOHM lock modifier to allow a call through the call explained in the function.