sherlock-audit / 2024-05-pooltogether-judging

8 stars 4 forks source link

0x73696d616f - The accumulated balance in the `TwabController` will overflow for tokens with a bigger number of decimals, leading to lost funds #97

Closed sherlock-admin4 closed 2 months ago

sherlock-admin4 commented 3 months ago

0x73696d616f

medium

The accumulated balance in the TwabController will overflow for tokens with a bigger number of decimals, leading to lost funds

Summary

The accumulated balance in the observations of the TwabController will overflow given enough time for tokens with enough decimal places, such as $ERBB, which has 27.

Vulnerability Detail

The observations in the TwabController are stored as uint128, as can be seen here. Thus, the maximum value is 2^128 - 1 ≃ 3.4e38.

Considering the $ERBB token, which has 27 decimals and a price of 1.31 USD at the time of writing, if 1 year passes, the remaining precision for the token balance is (2^128 - 1) / (365*24*3600) / 1e27 = 10800 tokens, or approximately 14148 USD at a price of 1.31.

Thus, with tokens such as $ERBB, the accumulated balance will overflow whenever users try to withdraw. It will not be possible to do anything as the accumulated balance is extrapolated to the current time, which means that it may not overflow when minting, but then overflow on burns due to the balance accruing over time.

As PrizeVaults can be built by anyone there is not any restriction on the tokens in scope and it is expected that users create vaults with tokens with more decimals than 18.

Impact

DoSed withdrawals and stuck funds due to the accumulated balance overflow.

Code Snippet

https://github.com/sherlock-audit/2024-05-pooltogether/blob/main/pt-v5-twab-controller/src/libraries/ObservationLib.sol#L30 https://github.com/sherlock-audit/2024-05-pooltogether/blob/main/pt-v5-twab-controller/src/libraries/TwabLib.sol#L412

Tool used

Manual Review

Vscode

Recommendation

The accumulated balance should be stored in a variable bigger than uint128.

Duplicate of #104

sherlock-admin2 commented 2 months ago

1 comment(s) were left on this issue during the judging contest.

infect3d commented:

low__ high decimal tokens are not common at all and all have low liquidity

nevillehuang commented 2 months ago

Invalid, this tokens are considered "weird tokens" that are not explicitly mentioned in contest details