code-423n4 / 2023-08-pooltogether-findings

4 stars 3 forks source link

Calculations like valueX8 and liquidity do not account for potential rounding errors #144

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/GeVault.sol#L268 https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/GeVault.sol#L277

Vulnerability details

Impact

Proof of Concept

The rounding errors in calculations like valueX8 and liquidity can accumulate over time and lead to inaccuracies.

The key areas where rounding errors can occur are:

  1. valueX8 calculation - Here the amount is multiplied by the asset price (a uint) and then divided by the token decimals (a power of 10). This can lead to loss of precision :https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/GeVault.sol#L268

  2. liquidity calculation - Here valueX8 and vaultValueX8 are multiplied and divided, again potentially losing precision : https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/GeVault.sol#L277

Over many deposits and withdrawals, these small rounding errors can accumulate and lead to the total liquidity minted being inaccurate compared to the actual assets deposited.

Tools Used

Manual

Recommended Mitigation Steps

calculations involving division should aim to keep the intermediate values in the highest precision possible before dividing.

Assessed type

Other

c4-pre-sort commented 1 year ago

raymondfam marked the issue as low quality report

raymondfam commented 1 year ago

A known QA issue in the bot.

c4-judge commented 1 year ago

HickupHH3 marked the issue as unsatisfactory: Insufficient proof