First Flight #3: Thunder Loan

Summary

Given the fact that LP tokens of AssetToken ERC20 contract are all 18 decimals by default with the current implementation, allowing a token with another number of decimals would result in a non 1-1 ratio for the creation of LP tokens with STARTING_EXCHANGE_RATE.

Vulnerability Details

3 of the tokens that could become allowed in the protocol have different number of decimals than 18 :

• USDT has 6 decimals

• USDC has 6 decimals

• ZIL has 12 decimals

In the case of USDC for example, depositing 1000 USDC is the same as passing 1000*1e6 for amount in the deposit function. mintAmount will be the same as amount before the first flashloan occurs, as:

Finally, LP tokens will be minted with mintAmount = 1000*1e6:

This means the liquidity provider will receive 1e9 of the LP tokens, which 0.000000001LP token. This could be really annoying and error prone. Ideally, the protocol should be able to mint a LP token with the same number of decimals.

Impact

The impact of this issue is MEDIUM as it generates error prone conversion rates with many power of ten between LP tokens and underlying tokens. It also induces many display issues. In the case of USDC and USDT, the difference between the balance of LP tokens and underlying tokens will be 1e12 factor. For ZIL, it will be 1e6.

Tools Used

Manual

Recommendations

I would suggest to allow modification of the number of decimals of a new AssetToken contract when it is deployed. That way, it could be easy to create a LP token with the same number of decimals as the underlying asset.

We could add another input parameter in AsssetToken constructor :

and assign i_numberOfDecimals = numberOfDecimals; after declaring a new immutable variable:

Then AssetToken should override decimals function of the ERC20 contract inherited :

Ultimately, ThunderLoan and ThunderLoanUpgraded contracts should modify the deployment of new AssetToken contracts to include the number of decimals:

I used the ERC20 wrapper after importing it because the IERC20 interface from Open Zeppelin doesn't classically include the decimals() function.