code-423n4 / 2021-09-yaxis-findings

0 stars 0 forks source link

Decimals of upgradeable tokens may change #82

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

pauliax

Vulnerability details

Impact

A theoretical issue is that the decimals of USDC may change as they use an upgradeable contract so you cannot assume that it stays 6 decimals forever: balances[1] = stableSwap3Pool.balances(1).mul(10**12); // USDC

Recommended Mitigation Steps

A simple solution would be to call .decimals() on token contract to query it on the go. Then you will not need to hardcode it but gas usage will increase.

Haz077 commented 2 years ago

I believe that there were other submissions mentioning issues with the decimal (for example: handling tokens with more than 18 decimal points). But I think calling .decimals() from the tokens contract will fix those issues, even though it will cost more gas, so I think this issue should be accepted.

uN2RVw5q commented 2 years ago

I would say that it's extremely unlikely that USDC will upgrade the contract and change its decimals, as this would break many existing contracts. Perhaps this should simply be documented. Even if it happens, it's very likely that this would be made public in advance, allowing the tokens to be withdrawn.

IMO, changing 10**12 to 10**18/USDC.decimals() is an unnecessary waste of gas (more than 5000 additional gas, even just counting the call, and storage read)

GalloDaSballo commented 2 years ago

I believe the finding to be valid, but wouldn't expect sponsor to have to mitigate