Skip to content
This repository has been archived by the owner on Dec 8, 2024. It is now read-only.

0xSpearmint1 - Malicious user can take advantage of auto-accruing WETH in the PrizePool contract on blast to unfairly increase winning odds #12

Closed
sherlock-admin3 opened this issue Jun 6, 2024 · 8 comments
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Escalation Resolved This issue's escalations have been approved/rejected High A valid High severity issue Reward A payout will be made for this issue

Comments

@sherlock-admin3
Copy link

sherlock-admin3 commented Jun 6, 2024

0xSpearmint1

high

Malicious user can take advantage of auto-accruing WETH in the PrizePool contract on blast to unfairly increase winning odds

Summary

Malicious user can take advantage of auto-accruing WETH in the PrizePool contract on blast to unfairly increase winning odds

Vulnerability Detail

The protocol has clearly stated they plan on deploying to Blast

They also state the following

We're interested to know if there will be any issues deploying the code as-is to any of these chains

On blast, smart contracts holding WETH have Automatic yield by default. It is auto-accruing, so over time the WETH balance of the contract will increase as yield comes in.

A user can take advantage of this by calling PrizePool.contributePrizeTokens() once the yield enters the contract, passing in their vault and the exact amount of blast WETH yield that just entered the contract.

function contributePrizeTokens(address _prizeVault, uint256 _amount) public returns (uint256) {
    uint256 _deltaBalance = prizeToken.balanceOf(address(this)) - accountedBalance();
    if (_deltaBalance < _amount) {
      revert ContributionGTDeltaBalance(_amount, _deltaBalance);
    }
    uint24 openDrawId_ = getOpenDrawId();
    _vaultAccumulator[_prizeVault].add(_amount, openDrawId_);
    _totalAccumulator.add(_amount, openDrawId_);
    emit ContributePrizeTokens(_prizeVault, openDrawId_, _amount);
    return _deltaBalance;
  }

Once the yield enter the contract the prizeToken.balanceOf(address(this)) will increase, therefore _deltaBalance will increase too. All an attacker has to do is wait for the yield, pre compute the _deltaBalance and then call contributePrizeTokens passing the calculated _deltaBalance as the _amount parameter

This is unfair because the WETH balance of the PrizePool contract is generated by all the users of the protocol. Hence the blast yield on that WETH should be distributed equitably, not to a single user.

Impact

A malicious user has unfairly increased their odds of winning at 0 cost at the expense of all other protocol users.

Code Snippet

https://github.com/sherlock-audit/2024-05-pooltogether/blob/1aa1b8c028b659585e4c7a6b9b652fb075f86db3/pt-v5-prize-pool/src/PrizePool.sol#L412-L422

Tool used

Manual Review

Recommendation

Implement a system so the blast yield is distributed in a fair way

Duplicate of #114

@github-actions github-actions bot added Medium A valid Medium severity issue Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label labels Jun 11, 2024
@nevillehuang nevillehuang added Medium A valid Medium severity issue and removed Medium A valid Medium severity issue labels Jun 15, 2024
@sherlock-admin3 sherlock-admin3 changed the title Creamy Ginger Salamander - Malicious user can take advantage of auto-accruing WETH in the PrizePool contract on blast to unfairly increase winning odds 0xSpearmint1 - Malicious user can take advantage of auto-accruing WETH in the PrizePool contract on blast to unfairly increase winning odds Jun 18, 2024
@sherlock-admin3 sherlock-admin3 added the Reward A payout will be made for this issue label Jun 18, 2024
@0xspearmint1
Copy link

This issue and it's duplicates should be a high severity issue

Root cause: Auto-accruing WETH in the PrizePool contract can be used by a single user to increase their chances of winning, decreasing all other user's chance of winning

Impact: Malicious actors can steal yield generated by the prize pool, inflating their vault's portion and unfairly increasing their win chances. This decreases legitimate players' portion and winning probabilities.

The protocol clearly stated in the README that they will deploy to blast

They also stated the following

We're interested to know if there will be any issues deploying the code as-is to any of these chains,

If the code is deployed as is to blast the impact is large enough to warrant high severity

@0xjuaan
Copy link

0xjuaan commented Jun 20, 2024

Escalate
On behalf of @0xspearmint1

@sherlock-admin3
Copy link
Author

Escalate
On behalf of @0xspearmint1

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

@sherlock-admin4 sherlock-admin4 added the Escalated This issue contains a pending escalation label Jun 20, 2024
@nevillehuang
Copy link
Collaborator

nevillehuang commented Jun 20, 2024

@trmid @elhajin @0xspearmint1

What is the numerical unfairness of winning chance increase considering the WETH rebasing. Also what is the % rebase per year for WETH?

Nevertheless I believe the escalation should be rejected since it doesn't provide any additional evidence and is simply repeating the issue.

@elhajin
Copy link
Collaborator

elhajin commented Jun 21, 2024

  • The yield for WETH comes from Lido staking , which yield about 4% APR. On Blast, it could be more since there will be a lot of ETH holders who will have the void mode , so their yield goes to others. let's assume a 6% yield in this case.

  • It's a bit complicated to estimate because we have different tiers that depends on contribution balances in differents periods for each tier. the grand prize for example, which appears once a year (not guaranteed since it's based on probabilities), if the PrizePool held 100 ETH in average for the year, 6 WETH would be stolen. The manipulation of winning probabilities means that the one who stole the yield will have 12% more chance to win the grandPrize (6% from the stolen yield and 6% from the decrease in others' chances).

  • However, it also depends on when this yield gets stolen and contributed , and if it's continuously get contributed or at once ...ect

  • for example If the yield is stolen between draw 340 and 360, and this is the TWAB period to calculate the winning zone for tier2 (20 days), the contribution of others in this period can be way less compared to the yield stolen , because the yield is for all the weth balance of prizePool since it's deployed, let's say other vaults contribution in this period is 1 WETH and the stolen yield is for 1 year is 6 WETH. So, the user who stole the yield in this case would have 6x chances to win tier2 prizes (not only one prize) compared to all other vaults combined in draw number 360

  • tbh It's impossible to estimate the manipulation of winning chances fully accurately due to the varying factors.but given that from the read me :

... The protocol requires that any integrations they create are not be able to manipulate their chances of winning unfairly.

It's fair to judge this issue as high severity.

@WangSecurity
Copy link

I agree that this warrants high severity, cause the users can increase their chances of winning for free and stealing the yield. But, I also agree that the escalation doesn't bring value why it's High and not Medium, since it just repeats what was said in the report. I understand that the rules don't require anything specific to write as a reason for escalation, but I believe it's understandable, that if you want to upgrade the severity from M to H, you should explain why, not summarise the report.

Planning to reject the escalation, but update the report to High severity (and duplicates).

And advice for future, if you want the upgrade severity of the entire family (or invalidate the entire family), please escalated the main issue, not a duplicate, it would be just easier for me, thank you in advance.

@WangSecurity WangSecurity added High A valid High severity issue and removed Medium A valid Medium severity issue labels Jun 24, 2024
@WangSecurity
Copy link

WangSecurity commented Jun 24, 2024

Result:
High
Duplicate of #114

@sherlock-admin2
Copy link
Contributor

sherlock-admin2 commented Jun 24, 2024

Escalations have been resolved successfully!

Escalation status:

@sherlock-admin3 sherlock-admin3 removed the Escalated This issue contains a pending escalation label Jun 24, 2024
@sherlock-admin4 sherlock-admin4 added the Escalation Resolved This issue's escalations have been approved/rejected label Jun 24, 2024
@WangSecurity WangSecurity added Medium A valid Medium severity issue High A valid High severity issue and removed High A valid High severity issue Medium A valid Medium severity issue labels Jul 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Escalation Resolved This issue's escalations have been approved/rejected High A valid High severity issue Reward A payout will be made for this issue
Projects
None yet
Development

No branches or pull requests

8 participants