turvec - The feeController differs from it's specification as setting 'applyFee' to false doesn't disable BOTH Dynamic pair-based and base fee but only Dynamic pair-based fee

[sherlock-admin]

turvec

medium

The feeController differs from it's specification as setting 'applyFee' to false doesn't disable BOTH Dynamic pair-based and base fee but only Dynamic pair-based fee

Summary

The feeController differs from it's specification as setting 'applyFee' to false doesn't disable BOTH Dynamic pair-based and base fee but only Dynamic pair-based fee

Vulnerability Detail

According to the contract's specification, setting 'applyFee' to false should disable both Dynamic pair-based fee and base fee

/// * Both dynamic and base fee can be disabled by setting 'applyFee' to false.

However, applyFee has a different usage in the code:

uint256 feeAmount = fee.applyFee
                ? order.outputs[i].amount.mulDivUp(fee.fee, DENOM)
                : order.outputs[i].amount.mulDivUp(baseFee, DENOM);

Instead, setting this value is used for specifying which fee is to used between both

Impact

Arbitrageurs and Aggregators that trade for expected output return opportunity on tokens with applyFee not set and therefore false with the belief that both fees are disabled due to the specification will receive incorrect return values when calling execute().

Code Snippet

https://github.com/sherlock-audit/2024-02-rubicon-finance/blob/main/gladius-contracts-internal/src/fee-controllers/RubiconFeeController.sol#L81C13-L83C68

Tool used

Manual Review

Recommendation

Ensure that the applyFee variable usage implemented in the code matches it's usage in their specifications.

Duplicate of #22

[sherlock-admin]

1 comment(s) were left on this issue during the judging contest.

0xAadi commented:

Invalid: The implementation is accurate, although the "@dev" comment appears to be a mistake.