Ironsidesec - Swap will revert if fee0 or fee1 is zero

[sherlock-admin2]

Ironsidesec

Medium

Swap will revert if fee0 or fee1 is zero

Summary

Revert on line 280 is not avoided on line 140 on right below. Previously on v3 pair, notifyRewardAmount is called only if amount > 0 on line 142 on left below. Now _sendTokenFees doesn't care if amount > or == 0, and this is an issue becasue swap will revert on cases listed in vulnerability section.

Scroll to line 141 to see the diff on https://www.diffchecker.com/ISBNwEPV/

https://github.com/sherlock-audit/2024-06-velocimeter/blob/63818925987a5115a80eff4bd12578146a844cfd/v4-contracts/contracts/ExternalBribe.sol#L280

ExternalBribe.sol

279:     function notifyRewardAmount(address token, uint amount) external lock {
280:  >>>    require(amount > 0);
281:         if (!isReward[token]) {
282:           require(IVoter(voter).isWhitelisted(token), "bribe tokens must be whitelisted");
283:           require(rewards.length < MAX_REWARD_TOKENS, "too many rewards tokens");
284:         }
... SNP ...
305:     }

Vulnerability Detail

Pair v4 and v3 has fee calculation similar, but the validation and placement of code is different in this flow.

1. The check if amount0In > 0 and hasGauge is same as v3. But if fee0 is > 0 check is done in v3 so that it wont revert on line 280 shown above in ExternalBribe.notifyRewardAmount.
2. Becasue there is a check if amount > 0 on _sendTokenFees in v3 pair, but on v4 pair, it calls notify reward without validation. And this causes swap to revert on certain like,
   • Gauge creation allows to create gauge if one of the tokens in pair is whitelisted (check here). So in these pairs, if the swap is called in a way fee0 or fee1 become 0, then thats wap will revert. Maybe some flashloan users get affected, low decimal tokens get affected.

Scroll to line 316 on https://www.diffchecker.com/ISBNwEPV/

Impact

Swap will revert if fee0 or fee1 is zero due tio missing check on _sendTokenFees. Since its a important function of system, its high impact if reverts but with low likelihood, So giving medium severity

Code Snippet

https://github.com/sherlock-audit/2024-06-velocimeter/blob/63818925987a5115a80eff4bd12578146a844cfd/v4-contracts/contracts/ExternalBribe.sol#L280

Scroll to line 141 on https://www.diffchecker.com/ISBNwEPV/

Scroll to line 316 on https://www.diffchecker.com/ISBNwEPV/

Tool used

Manual Review

Recommendation

https://github.com/sherlock-audit/2024-06-velocimeter/blob/63818925987a5115a80eff4bd12578146a844cfd/v4-contracts/contracts/Pair.sol#L150

    function _sendTokenFees(address token, uint amount) internal {
+       if(amount > 0) {
            IBribe(externalBribe).notifyRewardAmount(token, amount); // transfer fees to exBribes
            emit GaugeFees(token, amount, externalBribe);
+       }
    }

Duplicate of #52