First Flight #3: Thunder Loan

Summary

Whenever users want to take a flashloan in ThunderLoan contract (or ThunderLoanUpgraded contract), they need to pay a fee for this service. This fee is set in constructor and can be changed by the owner of ThunderLoan contract. The initial value is set as follows :

This means there is a 0.03% fee for each requested flashloan.
This fee represents a percentage of the deposited amount and needs to be repaid with the borrowed amount in the executeOperation callback function.

The calculation method of the fee is problematic. Indeed, it first converts the borrowed amount to its value in WEI:

This doesn't make sense as the fee is a fix percentage of the borrowed amount of token, and is paid in token, not in ether. There is no pool mechanism in this protocol.

Vulnerability Details

The following example will show why the current implementation of getCalculatedFee doesn't make sense :

• If i borrow 1000 tokens that are worth 0.01 ether each, my fee will be 0.3% of 1000*0.01 which is 0.03 tokens

• If i borrow 1000 tokens that are worth 10 ether each, my fee will be 0.3% of 1000 * 10 which is 30 tokens

• If i borrow 1000 tokens that are worth 1000 ether each (let's imagine), my fee will be 0.3% of 1000 * 1000 which is 3000 tokens, a 300% fee !

In the first situation, borrowing 1000 tokens costs 0.03 tokens, while it will cost 3000 tokens in the last one.This means that when a token is worth more ether, the fee will represent a bigger percentage of the borrowed tokens.

The cost of flashloan with the current implementation depends on the price in eth of the token to borrow, and this is not what is expected.
No matter the price of the token, the flashloan should cost 0.3% (with current rate) of the deposit (and it's value in ether).

Impact

The impact of this issue is HIGH as it leads to a wrong calculation of the fee owned by users taking flashloans to the protocol. The bigger the price in ether of the borrowed token is, the more expensive the flashloan will be, which is not logic. Instead, the correct logic should be : the more I borrow tokens, the more I pay fees, independently of the price of the token.

Tools Used

Manual

Recommendations

My recommandation is to remove OracleUpgradeable contract in the logic of ThunderLoan and ThunderLoanUpgraded contracts.
At no point ThunderLoan contract needs to deal with conversion between tokens and ether values.

The getCalculatedFee function should me modified to correctly implement the fee calculation :

This correction also removes the first input parameter IERC20 token which is not useful anymore.

The following line in flashloan function should also be corrected :

should be :