Base fees are still applied when applyFee is false

Summary

Fees for pairs are stored in a struct with a flag to decide whether to apply fees. When this is false a base fee is still applied, contrary to the comments.

Vulnerability Detail

The Natspec comment for RubiconFeeController states that "both dynamic and base fee can be disabled by setting 'applyFee' to false", but the fee calculation logic is different. This actually uses the fee set for the pair if applyFee is true, otherwise it uses the base fee.

Impact

Users are charged higher fees than they may expect, causing unintended loss of funds.

Code Snippet

https://github.com/sherlock-audit/2024-02-rubicon-finance/blob/11cac67919e8a1303b3a3177291b88c0c70bf03b/gladius-contracts-internal/src/fee-controllers/RubiconFeeController.sol#L81-L84

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

Tool used

Manual Review

Recommendation

Decide on the correct behaviour for fees and update either the calculation logic or comment accordingly.

Duplicate of #22

sherlock-audit / 2024-02-rubicon-finance-judging

m4ttm - Base fees are still applied when applyFee is false #78