First Flight #3: Thunder Loan
First Flight #3
Beginner FriendlyFoundryDeFiOracle
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

s_feePrecision of 1e18 will cause improper truncation for tokens with decimals of less than 18

happyformerlawyer

Summary

Some of the tokens that are identified as underlying tokens that can be loaned do not have 18 decimals of precision. For example, both USDT and USDC have 6 decimals of precision. But the contract sets s_feePrecision to 1e18 for all tokens. In getCalculatedFee when the amount is multiplied by the price of the underlying token in WETH and then divided by s_feePrecision, you would truncate several decimals of precision if you were using a token like USDT or USDC.

Vulnerability Details

function getCalculatedFee(
IERC20 token,
uint256 amount
) public view returns (uint256 fee) {
//slither-disable-next-line divide-before-multiply
uint256 valueOfBorrowedToken = (amount *
getPriceInWeth(address(token))) / s_feePrecision;
//slither-disable-next-line divide-before-multiply
fee = (valueOfBorrowedToken * s_flashLoanFee) / s_feePrecision;

Impact

You could end up with a much smaller fee calculation than you should or, worse, you could end up with something less than 1 (if the amount of the flash loan were small enough) which would round down to 0. Depositors would not get as much fees (or any fees) to compensate them for their deposits when flash loans were made.

Tools Used

Manual review

Recommendations

Instead of s_feePrecision, add a mapping of token to decimals that will allow you to set the correct number of decimals when you call setAllowedToken to add a token. Then call that mapping to get the correct number of decimals when needed. You can do this with the following changes:

Add a mapping of token to decimals:

mapping(IERC20 => uint256) public s_tokenToDecimals;

In setAllowedToken, add another parameter for uint256 decimals:

function setAllowedToken(
IERC20 token,
bool allowed,
uint256 decimals
)

Push the correct number of decimals to the mapping in setAllowedToken:

s_tokenToDecimals[token] = decimals;

Refactor getCalculatedFee as follows. Note that in a different finding I point out that you shouldn't be involving getPriceInWeth in this calculation but I am leaving it here for purposes of this refactoring:

function getCalculatedFee(
IERC20 token,
uint256 amount
) public view returns (uint256 fee) {
uint256 valueOfBorrowedToken = (amount *
getPriceInWeth(address(token))) / s_tokenToDecimals[token];
fee = (valueOfBorrowedToken * s_flashLoanFee) / s_tokenToDecimals[token];
}

Refactor the getFeePrecision function:

function getFeePrecision(IERC20 token) external view returns (uint256) {
return s_tokenToDecimals[token];
}

Refactor updateFlashLoanFee:

function updateFlashLoanFee(uint256 newFee) external onlyOwner {
if (newFee > 1e18) {
revert ThunderLoan__BadNewFee();
}
s_flashLoanFee = newFee;
}
Updates

Lead Judging Commences

0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

flashloan with differing fees/prices for different decimal tokens

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.