FarmStrategy Manual Review Findings

FSY-01M: Inexistent Guarantee of Fee Capture

Type	Severity	Location
Logical Fault		FarmStrategy.sol: • I-1: L562, L567 • I-2: L599, L604 • I-3: L647, L651

Description:

The data entry utilized to capture the relevant fees in the FarmStrategy contract can be arbitrarily defined by the contract's caller, permitting fees to effectively be bypassed by supplying unused tokens or an empty array.

Impact:

Fees can be presently bypassed via carefully crafted interaction payloads with the FarmStrategy implementation.

Example:

Copy
560targets[2] = address(feesLib);
561data[2] = abi.encodeCall(
562    IFeesLib.chargeFees, (strategyAddress, fee, params.tokensOut)
563);
564
565targets[3] = address(transferLib);
566data[3] =
567    abi.encodeCall(ITransferLib.transferTokensToUser, (sweepTokens));

Recommendation:

We advise the data entry utilized for fee capturing to be sanitized, either by utilizing on-chain data to craft it or by utilizing an off-chain signature that ensures the payload has been securely generated by the vfat team's front-end; we consider either of the proposed approaches as adequate in rectifying this exhibit.

Alleviation (6ab7af3bb495b817ffec469255ea679b1813eecb):

The vfat team evaluated this exhibit and while they consider it to be valid, they do not believe it constitutes an active risk toward users and thus wish to address it at a later date.

As the assessment of the vfat team is correct and the sole impact of the exhibit is toward the protocol itself, we consider this exhibit to have been safely acknowledged.

FSY-02M: Swap-Based Fee Bypassing

Type	Severity	Location
Logical Fault		FarmStrategy.sol: • I-1: L557-L558, L560-L563 • I-2: L594-L595, L597-L600

Description:

The FarmStrategy::_harvest and FarmStrategy::_withdraw functions will execute swaps (ISwapLib::swapMUltiple and IZapLib::zapOut respectively) before charging the relevant fees through the IFeesLib::chargeFees function, permitting fees to be effectively bypassed by emptying out the relevant tokensOut balances for alternative assets that are ultimately withdrawn through sweepTokens.

Impact:

It is presently possible to bypass all complex harvest and withdrawal fees by performing swaps toward tokens that are not part of a strategy's tokensOut configuration.

Example:

Copy
572function _withdraw(
573    Sickle sickle,
574    Farm calldata farm,
575    WithdrawParams calldata params,
576    address[] calldata sweepTokens,
577    bytes4 fee
578) private {
579    address[] memory targets = new address[](4);
580    bytes[] memory data = new bytes[](4);
581
582    address farmConnector =
583        connectorRegistry.connectorOf(farm.stakingContract);
584    targets[0] = farmConnector;
585    data[0] = abi.encodeCall(
586        IFarmConnector.withdraw,
587        (
588            farm,
589            params.zap.removeLiquidityParams.lpAmountIn,
590            params.extraData
591        )
592    );
593
594    targets[1] = address(zapLib);
595    data[1] = abi.encodeCall(IZapLib.zapOut, (params.zap));
596
597    targets[2] = address(feesLib);
598    data[2] = abi.encodeCall(
599        IFeesLib.chargeFees, (strategyAddress, fee, params.tokensOut)
600    );
601
602    targets[3] = address(transferLib);
603    data[3] =
604        abi.encodeCall(ITransferLib.transferTokensToUser, (sweepTokens));
605
606    sickle.multicall(targets, data);
607}

Recommendation:

We advise the code to ensure that fees are charged before performing any swaps, ensuring that they are properly captured and cannot be bypassed.

Alleviation (6ab7af3bb495b817ffec469255ea679b1813eecb):

The vfat team evaluated this exhibit and while they consider it to be valid, they do not believe it constitutes an active risk toward users and thus wish to address it at a later date.

As the assessment of the vfat team is correct and the sole impact of the exhibit is toward the protocol itself, we consider this exhibit to have been safely acknowledged.