sherlock-audit / 2023-06-tokemak-judging

10 stars 8 forks source link

feelereth - The decimalPad will remain the same even if token decimals changes while the ratio calculation will be incorrect. #636

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

feelereth

high

The decimalPad will remain the same even if token decimals changes while the ratio calculation will be incorrect.

Summary

An outdated decimalPad variable can lead to calculation errors and bugs,

Vulnerability Detail

The decimalPad variable is set once at deployment based on the decimals of the token. For example, if the token has 18 decimals, decimalPad would be 10^18. This decimalPad is then used in the ratio calculation Link The issue is that if the token decimal ever changes after deployment, the decimalPad will remain the old value but the ratio calculation will be incorrect. For example, if the token originally had 18 decimals, decimalPad is 10^18. If the token decimal later changes to 6, the ratio calculation will still use decimalPad=10^18, even though it should be 10^6. This will make the ratio off by a factor of 10^12. So in summary: • decimalPad set once at deployment based on original token decimals • Used in ratio calculation to scale assets/liabilities to token units • If token decimals change later, ratio calculation will be off

Impact

Code Snippet

https://github.com/sherlock-audit/2023-06-tokemak/blob/main/v2-core-audit-2023-07-14/src/beacon/FrxBeaconChainBacking.sol#L16 https://github.com/sherlock-audit/2023-06-tokemak/blob/main/v2-core-audit-2023-07-14/src/beacon/FrxBeaconChainBacking.sol#L34 https://github.com/sherlock-audit/2023-06-tokemak/blob/main/v2-core-audit-2023-07-14/src/beacon/FrxBeaconChainBacking.sol#L49

Tool used

Manual Review

Recommendation

A. Don't make decimalPad immutable, allow it to be changed B. Update decimalPad whenever the token decimals change C. Require the token decimals to be immutable once set D. Perform the decimal conversion dynamically based on current decimals

sherlock-admin2 commented 1 year ago

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

Trumpero commented:

invalid, if the tokens "can" change their decimal, the protocol can simply deploy a new contract for that new decimal