FlashloanStrategy Manual Review Findings

FSG-01M: Argument-Based Decoding Dependency

Type	Severity	Location
Logical Fault		FlashloanStrategy.sol: • I-1: L287-L288 • I-2: L301-L302

Description:

The Uniswap V2 and Uniswap V3 flash-loan mechanisms rely on specific arguments being supplied to the penultimate callback call executed by the flash-loan operation, which based on the current codebase's state is either the FlashloanLib::flashloanDepositCallback or the FlashloanLib::flashloanWithdrawCallback function implementations.

While the FlashloanStrategy indicates that it may be compatible with multiple callback selectors, the arguments that the selectors must possess for Uniswap-like flash-loans are restricted to the aforementioned ones (or greater).

Impact:

If any other callback function is supported that does not contain the arguments denoted in the decoding signature, the Uniswap callback code will either revert or capture corrupt premiums.

Example:

Copy
287(,, uint256[] memory premiums,) =
288    abi.decode(params[4:], (address[], uint256[], uint256[], bytes));

Recommendation:

We advise the code to either ensure that the callbacks invoked are the ones presently implemented by the FlashloanLib if a Uniswap flash-loan is performed, or the codebase to outline this argument-based dependency in the code itself to illustrate that the Uniswap calls are solely compatible with the current FlashloanLib implementation.

Alleviation (6ab7af3bb495b817ffec469255ea679b1813eecb):

The vfat team evaluated this exhibit and clarified that they intend to scrutinize flash loan strategy deployments to ensure that the decoding requirements are met.

As such, we consider this exhibit to have been safely acknowledged.

FSG-02M: Non-Standard Fee Evaluation Mechanism

Type	Severity	Location
Logical Fault		FlashloanStrategy.sol:L687

Description:

The Uniswap V3 (and Pancakeswap V3) fee calculation mechanism will presently rely on a providerFee configured via its input arguments rather than extracting data from the protocol integrated itself.

As fees are ignored in the Uniswap-related callbacks and the result of the FlashloanStrategy::calculatePremiums function is relied on instead, the premiums supplied might end up being greater than the actual ones expected by the AMM pair.

Impact:

An incorrect flash-loan provider fee configured for the Uniswap V3 mechanism will result in either failed transactions or an oversuppliance of funds, either of which is an undesirable outcome.

Example:

Copy
682} else if (providerType == FlashloanProvider.UNIV3) {
683    uint256 length = amounts.length;
684    for (uint256 i; i < length;) {
685        if (amounts[i] > 0) {
686            premiums[i] =
687                ((amounts[i] * providerFee) / 10_000 / 100) + 1; // hundredths
688                // of basis points
689        } else {
690            premiums[i] = 0;
691        }
692
693        unchecked {
694            ++i;
695        }
696    }
697}

Recommendation:

We advise the code to instead evaluate the flash-loan premiums by supplying the token addresses of the AMM pool the flash-loan is acquired from rather than the providerFee itself, permitting the code to query and calculate the fee directly by using on-chain data.

Alleviation (6ab7af3bb495b817ffec469255ea679b1813eecb):

The vfat team evaluated this exhibit and has opted to acknowledge it.

We would like to note that the impact of this exhibit is non-negligible, and transaction failures as well as improper premiums might be observed.

In any case, we consider the exhibit to have been acknowledged in accordance with the intentions of the vfat team.

FSG-03M: Improper Premium Rounding Operations

Type	Severity	Location
Mathematical Operations		FlashloanStrategy.sol: • I-1: L619-L622 • I-2: L637-L640 • I-3: L673 • I-4: L687

Description:

The premium rounding operations across the Aave V2, Aave V3, Uniswap V2, and Uniswap V3 protocols are presently incorrect as they do not adhere to their respective protocol implementations.

Impact:

The FlashloanStrategy::calculatePremiums function will consistently overvalue the premiums that are expected for multiple flash-loan protocols due to incorrect rounding operation implementations.

Example:

Copy
613if (providerType == FlashloanProvider.AAVEV2) {
614    uint256 aaveV2FlashloanPremiumInBasisPoints =
615        ILendingPoolV2(aaveV2LendingPool).FLASHLOAN_PREMIUM_TOTAL();
616    uint256 length = amounts.length;
617    for (uint256 i; i < length;) {
618        if (amounts[i] > 0 && aaveV2FlashloanPremiumInBasisPoints > 0) {
619            premiums[i] = (
620                (amounts[i] * aaveV2FlashloanPremiumInBasisPoints - 1)
621                    / 10_000
622            ) + 1;
623        } else {
624            premiums[i] = 0;
625        }
626
627        unchecked {
628            ++i;
629        }
630    }
631} else if (providerType == FlashloanProvider.AAVEV3) {

Recommendation:

We advise the Aave V2 premium calculation to round downward, the Aave V3 premium calculation to round to the nearest neighbour, and the Uniswap V2/V3 implementations to round properly by subtracting 1 from their respective denominators.

Alleviation (986c6b0a71):

The vfat team revisited this exhibit and applied two out of the four recommendations; namely, the Aave V2 and Aave V3 rounding operations were corrected yet the Uniswap V2 and Uniswap V3 continue to round incorrectly.

The Uniswap V2 and Uniswap V3 calculations will add 1 without subtracting 1 from the numerator, resulting in an overestimation by 1 unit if the numerator and divider match precisely.

Alleviation (73206ba3b5):

A subtraction by 1 was correctly introduced in the Uniswap V2 and Uniswap V3 premium calculations, alleviating this exhibit in full.

FSG-04M: Potentially Incorrect Flash-Loan Amounts

Type	Severity	Location
Logical Fault		FlashloanStrategy.sol: • I-1: L285-L286, L295 • I-2: L299-L300, L309

Description:

The Uniswap V2 and Uniswap V3 flash-loan implementations are presently incorrect as they do not validate that the two assets are provided in an ordered manner to the flash-loan request.

As the IUniswapV2Factory::getPair and IUniswapV3Factory::getPair functions will yield an AMM pool regardless of the order the tokens are supplied in, the actual amounts requested in the flash-loan will be incorrect if assets[0] is not lower than assets[1].

Impact:

All Uniswap V2 and V3 flash-loans will acquire incorrect amounts if the tokens are not properly ordered in the flash-loan request.

Example:

Copy
283} else if (providerType == FlashloanProvider.UNIV2) {
284    if (assets.length != 2) revert NotAnAssetPair();
285    address poolAddress = IUniswapV2Factory(quickswapFactoryAddr)
286        .getPair(assets[0], assets[1]);
287    (,, uint256[] memory premiums,) =
288        abi.decode(params[4:], (address[], uint256[], uint256[], bytes));
289    bytes memory uniswapFlashParams = abi.encode(
290        sickleAddress, poolAddress, assets, amounts, premiums, params
291    );
292    // storing the hash of the callback data for safety checks
293    flashloanDataHash = keccak256(uniswapFlashParams);
294    IUniswapV2Pair(poolAddress).swap(
295        amounts[0], amounts[1], address(this), uniswapFlashParams
296    );
297} else if (providerType == FlashloanProvider.UNIV3) {
298    if (assets.length != 2) revert NotAnAssetPair();
299    address poolAddress = IUniswapV3Factory(uniswapV3FactoryAddr)
300        .getPool(assets[0], assets[1], providerFee);
301    (,, uint256[] memory premiums,) =
302        abi.decode(params[4:], (address[], uint256[], uint256[], bytes));
303    bytes memory uniswapFlashParams = abi.encode(
304        sickleAddress, poolAddress, assets, amounts, premiums, params
305    );
306    // storing the hash of the callback data for safety checks
307    flashloanDataHash = keccak256(uniswapFlashParams);
308    IUniswapV3Pool(poolAddress).flash(
309        address(this), amounts[0], amounts[1], uniswapFlashParams
310    );
311} else if (providerType == FlashloanProvider.MORPHO) {

Recommendation:

We advise this restriction to be upheld so as to ensure that the Uniswap flash-loan amounts are correctly requested in the appropriate token format.

Alternatively, we advise the amounts data entries to be swapped if the assets are not ordered to correct the amounts automatically.

Alleviation (6ab7af3bb495b817ffec469255ea679b1813eecb):

The code was updated to ensure a proper input assets order for Uniswap V2 and Uniswap V3 flash loans, alleviating this exhibit in full.