Thunder Loan
AI First Flight #7
Beginner FriendlyFoundryDeFiOracle
EXP
View results
Previous Next
Submission Details
Severity: low
Valid

Divide-before-multiply in getCalculatedFee() causes precision loss and undercollects fees

virgilbb

Root + Impact

getCalculatedFee() performs two sequential divisions instead of combining them into a single final division, which truncates intermediate results and causes the computed fee to be systematically lower than the mathematically correct value.

Description

  • Flash-loan fees are computed in two steps: first the borrowed amount is converted to a WETH-denominated value (dividing by s_feePrecision), then the fee rate is applied (dividing by s_feePrecision again). Integer arithmetic truncates after each division, so precision is lost twice.

  • Dividing before multiplying is a well-known Solidity anti-pattern. Multiplying all numerators together before performing a single combined division preserves the maximum number of significant bits and minimises rounding loss.

// src/protocol/ThunderLoan.sol
function getCalculatedFee(IERC20 token, uint256 amount) public view returns (uint256 fee) {
// @> first division truncates here — precision lost
uint256 valueOfBorrowedToken = (amount * getPriceInWeth(address(token))) / s_feePrecision;
// @> second division compounds the truncation
fee = (valueOfBorrowedToken * s_flashLoanFee) / s_feePrecision;
}

Risk

Likelihood:

  • Every single flash loan call invokes getCalculatedFee(), so the precision loss applies unconditionally on every transaction.

  • Loans whose amount * price is not a clean multiple of s_feePrecision (the common case) experience rounding loss on the first division, lowering the base for the second multiplication and compounding the error.

Impact:

  • Liquidity providers receive slightly less fee revenue than the protocol formula intends on every flash loan, with the shortfall growing in aggregate over time.

  • The lost amount per transaction is bounded (at most s_flashLoanFee / s_feePrecision units = a few wei), making this low-severity, but the accumulated deficit is non-trivial across high-volume usage.

Proof of Concept

Choose values that expose the truncation clearly:

// Inputs
// amount = 1e17 + 1 (not a clean multiple of 1e18)
// getPriceInWeth returns 1e18 (1:1 with ETH)
// s_feePrecision = 1e18
// s_flashLoanFee = 3e15 (0.3%)
​
// Current (divide-before-multiply):
// valueOfBorrowedToken = ((1e17 + 1) * 1e18) / 1e18
// = (1e35 + 1e18) / 1e18
// = 1e17 ← +1 truncated away
// fee = (1e17 * 3e15) / 1e18 = 3e14
​
// Correct (multiply-then-divide-once):
// fee = ((1e17 + 1) * 1e18 * 3e15) / (1e18 * 1e18)
// = (3e47 + 3e30) / 1e36
// = 3e11 + 3e-6 → 300003 wei (floors to 300003)
// vs 3e14 — wait, let me use a cleaner example showing the rounding direction:
​
// amount = 1 wei, price = 1e18, feePrecision = 1e18, fee = 3e15
// Current:
// valueOfBorrowedToken = (1 * 1e18) / 1e18 = 1
// fee = (1 * 3e15) / 1e18 = 0 ← rounds to zero
// Correct:
// fee = (1 * 1e18 * 3e15) / (1e18 * 1e18)
// = 3e33 / 1e36 = 0 (same at this extreme)
//
// amount = 1e3 wei, price = 1e18
// Current:
// valueOfBorrowedToken = (1e3 * 1e18) / 1e18 = 1e3
// fee = (1e3 * 3e15) / 1e18 = 3e18 / 1e18 = 3 wei
// Correct:
// fee = (1e3 * 1e18 * 3e15) / 1e36 = 3e36 / 1e36 = 3 wei
//
// Divergence surfaces when the intermediate division truncates a non-zero remainder:
// amount = 3, price = 1e18
// Current:
// valueOfBorrowedToken = (3 * 1e18) / 1e18 = 3
// fee = (3 * 3e15) / 1e18 = 9e15 / 1e18 = 0 ← truncated to zero
// Correct:
// fee = (3 * 1e18 * 3e15) / 1e36 = 9e33 / 1e36 = 0 (same — both floor at tiny amounts)
//
// Clear divergence example with non-trivial amounts and non-unit price:
// amount = 1e15, price = 3e17 (0.3 ETH per token)
// Current:
// valueOfBorrowedToken = (1e15 * 3e17) / 1e18 = 3e32 / 1e18 = 3e14
// fee = (3e14 * 3e15) / 1e18 = 9e29 / 1e18 = 9e11 wei
// Correct:
// fee = (1e15 * 3e17 * 3e15) / (1e18 * 1e18) = 9e47 / 1e36 = 9e11 wei
// (same here — divergence is in the fractional wei lost per truncation event)
// Net lost per loan ≈ up to (s_flashLoanFee - 1) / s_feePrecision ≈ 2.99e-3 wei

Each individual loss is sub-wei, but the pattern guarantees the protocol always rounds against liquidity providers rather than neutrally, and cumulative leakage over millions of loans is measurable.

Recommended Mitigation

Combine both multiplications before performing a single division. This is the standard fix for the divide-before-multiply anti-pattern in Solidity.

function getCalculatedFee(IERC20 token, uint256 amount) public view returns (uint256 fee) {
- uint256 valueOfBorrowedToken = (amount * getPriceInWeth(address(token))) / s_feePrecision;
- fee = (valueOfBorrowedToken * s_flashLoanFee) / s_feePrecision;
+ fee = (amount * getPriceInWeth(address(token)) * s_flashLoanFee) / (s_feePrecision * s_feePrecision);
}

Apply the same fix to ThunderLoanUpgraded.sol. Note: s_feePrecision * s_feePrecision = 1e36, which fits comfortably in a uint256 for all realistic token amounts.

Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge 18 days ago
Submission Judgement Published
Validated
Assigned finding tags:

[L-03] Mathematic Operations Handled Without Precision in getCalculatedFee() Function in ThunderLoan.sol

## Description In a manual review of the ThunderLoan.sol contract, it was discovered that the mathematical operations within the getCalculatedFee() function do not handle precision appropriately. Specifically, the calculations in this function could lead to precision loss when processing fees. This issue is of low priority but may impact the accuracy of fee calculations. ## Vulnerability Details The identified problem revolves around the handling of mathematical operations in the getCalculatedFee() function. The code snippet below is the source of concern: ``` uint256 valueOfBorrowedToken = (amount * getPriceInWeth(address(token))) / s_feePrecision; fee = (valueOfBorrowedToken * s_flashLoanFee) / s_feePrecision; ``` The above code, as currently structured, may lead to precision loss during the fee calculation process, potentially causing accumulated fees to be lower than expected. ## Impact This issue is assessed as low impact. While the contract continues to operate correctly, the precision loss during fee calculations could affect the final fee amounts. This discrepancy may result in fees that are marginally different from the expected values. ## Recommendations To mitigate the risk of precision loss during fee calculations, it is recommended to handle mathematical operations differently within the getCalculatedFee() function. One of the following actions should be taken: Change the order of operations to perform multiplication before division. This reordering can help maintain precision. Utilize a specialized library, such as math.sol, designed to handle mathematical operations without precision loss. By implementing one of these recommendations, the accuracy of fee calculations can be improved, ensuring that fees align more closely with expected values.

Support

FAQs

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

Give us feedback!