Summary

In the ThunderLoanUpgraded contract, the variables defined are not in the same order as defined in the initial implementation of ThunderLoan. As a result of which the variables defined in the upgraded contract will correspond to the incorrect storage slot which was set up in the initial implementation in respect with their variables. Thus, it will return incorrect values for the variables as it will read the values for incorrect storage slot due to change in the order of variables which in turn will have different storage slot corresponding to them.

Vulnerability Details

The upgraded contract have different order in which variables are defined which will lead to incorrect correspondence between storage slots which was set up in the initial implementation and thus will have an impact to read/write of the contract as the variables now have different storage slots. This will result in incorrect fees calculation.

Here as the s_flashLoanFee has a different storage slot in upgraded contract (Storage Slot 1) which corresponds to s_feePrecision of initial implementation contract. Thus, the value of s_flashLoanFee will return the value for s_feePrecision.
Also, the mapping s_currentlyFlashLoaning will be effected as it has storage slot 2 (earlier, storage slot 3).
As a result of which values will be mapped and retrieved on the basis of storage slot 2, thus will return incorrect values as correct storage slot was 3.

PoC

Add the test in test/unit/ThunderLoanTest.t.sol

Also include: import { ThunderLoanUpgraded } from "../../src/upgradedProtocol/ThunderLoanUpgraded.sol";

Run the test: forge test --mt test_UpgradeContract_ButWithIncorrectRetrievalOfValues

Impact

Incorrect read/write of data, resulting in incorrect calculations related to fees.

Tools Used

Manual Review, Foundry Tests

Recommendations

To have the same exact variables used in the previous implementation and in same order.