getCalculatedFee converts the loan into WETH value and then charges that number as a quantity of the borrowed token

Description

• The fee is meant to be 0.3% of the loan — s_flashLoanFee = 3e15 against s_feePrecision = 1e18, and the code comment says "0.3% ETH fee".

• The function first converts the borrowed amount into its WETH value, then applies the 0.3% rate to that value. The result is a number denominated in WETH — but it is returned as fee and compared against the balance of the borrowed token:

The units only cancel when getPriceInWeth(token) == 1e18, i.e. when the token happens to trade at exactly one WETH. For anything else the effective rate is 0.3% × price, so it scales linearly with how expensive the token is against ETH:

USDC, USDT, ZIL and KNC are all named in the README compatibility list, so these are supported markets rather than hypothetical ones. All seven tokens in that list trade below 1 WETH, so every supported market is under-charged — there is no configuration in which the protocol collects its advertised rate.

To be precise about causation, because it is easy to attribute this to the wrong thing: getCalculatedFee is linear in amount, so the token's decimals cancel out of the ratio. actual / intended = price / 1e18, and nothing else. The 2000× under-charge below is entirely the price factor, not the 6 decimals. Decimals are a second, separate defect in the same line — valueOfBorrowedToken divides a 6-decimal amount by 1e18, so the intermediate is meaningless as a WETH value — but they do not drive the ratio.

The same defect is present unchanged in src/upgradedProtocol/ThunderLoanUpgraded.sol:244-249, so the upgrade does not fix it.

Risk

Likelihood:

• Every flash loan of any token that does not trade at exactly 1 WETH — which is every token in practice, and certainly all seven named in the compatibility list. No attacker and no precondition; the miscalculation is the normal path.

• Nothing constrains it. There is no decimals normalisation anywhere in the contract, and getCalculatedFee is public view, so a borrower can read the near-zero quote before deciding to borrow, which makes it self-selecting: the cheapest markets attract the most volume.

Impact:

• Liquidity providers earn essentially nothing on stablecoin markets, which is where flash-loan demand actually is. On a 1,000,000 USDC loan the protocol collects 1.5 USDC where it intended to collect 3,000 — a factor of 2000. The capital is fully at risk for the duration of the loan and is compensated at 0.00015%.

• The protocol has exactly one revenue mechanism and this nullifies it for the tokens that matter, so the LP incentive the README describes ("AssetTokens gain interest over time depending on how often people take out flash loans") does not hold.

• In the other direction the error over-charges for any token worth more than ETH, so those borrowers pay above the advertised rate.

• Nothing about this is a rounding artifact or an edge case: it is a systematic mis-collection on every loan of every listed token, and it silently transfers the protocol's entire revenue stream to borrowers. Impact High × Likelihood High ⇒ High.

• Why the project's own tests cannot see it: test/mocks/MockTSwapPool.sol:5-7 returns a hard-coded 1e18. The suite therefore only ever exercises the single price at which the formula happens to be correct.

Proof of Concept

A 6-decimal token priced at 0.0005 WETH — USDC with ETH around 2,000.

Save as test/PoC_H06.t.sol and run forge test --mt test_H06 -vv:

Output:

On the "flat fee of the token price" comment

The NatSpec above the fee variables reads "The fee in WEI, it should have 18 decimals. Each flash loan takes a flat fee of the token price." It is worth addressing directly, because it could be read as documenting this behaviour as intentional.

It cannot be, for three reasons. First, the same comment calls the value a "0.3% ETH fee", and 0.3% of the loan is what the fee actually is only when the price is 1e18 — the two halves of the comment agree only in that one case. Second, "should have 18 decimals" is a statement about s_flashLoanFee's own scaling, not about the units of the output. Third, a fee that is deliberately proportional to a token's ETH price would mean a borrower of a cheap token pays proportionally less for identical risk and identical use of the same pool, which no fee schedule is designed to do, and which the README's LP-yield model contradicts.

Relationship to the oracle finding

I am separately reporting that the TSwap spot price this function reads is attacker-manipulable. The two are distinct and neither is a subset of the other:

• Replacing the spot price with a TWAP or a Chainlink feed does nothing here — a correct, unmanipulated price of 5e14 still produces a 1.5 USDC fee on a 1,000,000 USDC loan. This defect is in the formula, not in the source.

• Conversely, fixing the units while keeping an ETH-denominated fee leaves the price manipulable.

The overlap is real in one direction only, and I would rather name it than have it found: if the project adopts the percentage-fee mitigation below, the price lookup disappears and both findings are closed by that single edit.

Recommended Mitigation

Charge the rate against the borrowed amount and drop the price conversion. The price lookup is not needed for a percentage fee at all — that is what makes this a clean fix rather than a trade-off.

Removing the oracle from this path also removes the only place the TSwap price is consumed, which independently eliminates the manipulation surface in OracleUpgradeable::getPriceInWeth.

If the fee genuinely must be ETH-denominated — for instance to charge a fixed dollar amount per loan — then it has to be converted back into token units before it is compared against a token balance, and normalised for decimals:

Whichever direction is chosen, add a test that asserts the effective rate is the same for an 18-decimal token and a 6-decimal token; the current suite has no fee-calculation test at all.