Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inflation attack by drip #24

Closed
code423n4 opened this issue May 15, 2023 · 3 comments
Closed

Inflation attack by drip #24

code423n4 opened this issue May 15, 2023 · 3 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-3 satisfactory satisfies C4 submission criteria; eligible for awards sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")

Comments

@code423n4
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2023-05-xeth/blob/d86fe0a9959c2b43c62716240d981ae95224e49e/src/wxETH.sol#L87

Vulnerability details

Impact

The drip might inflate the exchange rate on an initial stake such that that subsequent stakers get minted zero wxETH. Their stake can then be unstaked by the first staker, together with their own first stake and inflation investment. Effectively, the first staker might be able to steal the second stake.

Proof of Concept

The underlying principle is the same as the well-known inflation attack, where the exchange rate is inflated by a direct token transfer, which this contract is also vulnerable to. Refer to my separate report, titled "Inflation attack by token transfer
", on that issue for details, if needed.
The difference here is that the inflation is instead caused by the drip.

Alice has the sole stake of 1 xETH, and owns the total supply of 1 wxETH (she can achieve this either by staking only this amount or by unstaking a large amount). The contract drips 1e18 - 1 xETH. The exchange rate is now (cashMinusLocked * BASE_UNIT) / _totalSupply = 1e18 * 1e18 / 1 = 1e36.
When Bob next stakes b xETH, he will get (b * BASE_UNIT) / exchangeRate() = b / 1e18 wxETH. This suffers significant rounding losses; up to 1e18 - 1 xETH from b is rounded off. Especially, if b < 1e18 Bob gets nothing in return for his stake.
These rounding losses are still staked, and therefore claimable by Alice when she unstakes.

Note that if b == 2e18 - 1, Bob will get 1 wxETH. The exchange rate will now be (3e18 - 1) * 1e18 / 2 = 1.5e36 - 0.5e18. When Carol stakes c xETH, she will get c * 1e18 / (1.5e36 - 0.5e18) wxETH. So if c < 1.5e18 she will, like Bob in the first case above, not get any wxETH for her xETH. So if Carol stakes 1.5e18 - 1 xETH, Alice and Bob can then each unstake their 1 wxETH and get about (3e18 - 1 + 1.5e18 - 1) / 2 = 2.25e18 - 1 xETH each. This is a profit of 2.25e18 - 2 xETH for Alice, and a 0.25e18 xETH profit for Bob.
This demonstrates that the rounding losses persist and continue to benefit (primarily) Alice.

Suggested Mitigation Steps

Dripping is currently independent of staked amount. A cap may be put in place which restricts dripping which would make the exchange rate too high.
The drip does not have to proportional to totalSupply() when it is large and rounding errors are not an issue, but if it were proportional at least at the low end, it would in a way be proportional to rounding losses, which should eliminate the issue.
Rounding losses could be prohibited by only allowing stakes in amounts where no rounding loss occur. However, stakers would then still be forced to choose amounts in large discrete steps.

Assessed type

ERC4626

@code423n4 code423n4 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels May 15, 2023
code423n4 added a commit that referenced this issue May 15, 2023
@c4-judge
Copy link
Contributor

kirk-baird marked the issue as duplicate of #3

@c4-sponsor
Copy link

vaporkane marked the issue as sponsor confirmed

@c4-sponsor c4-sponsor added the sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") label May 23, 2023
@c4-judge
Copy link
Contributor

kirk-baird marked the issue as satisfactory

@c4-judge c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label May 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-3 satisfactory satisfies C4 submission criteria; eligible for awards sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")
Projects
None yet
Development

No branches or pull requests

3 participants