Rich Chrome Whale - Wrong chek in `UniswapImplementation::removeFeeExemption` and `UniswapImplementation::geFee`making exemptionFee useless

[sherlock-admin4]

Rich Chrome Whale

Medium

Wrong chek in UniswapImplementation::removeFeeExemption and UniswapImplementation::geFeemaking exemptionFee useless

Summary

Broken core contract functionality In UniswapImplementation::removeFeeExemption and UniswapImplementation::getFee, we first have to check if there is an exemption fee in above functions but the check is done wrongly making this check is always false.

Root Cause

In removeFeeExemption UniswapImplementation.sol#L752-L753

File: UniswapImplementation.sol
749:     function removeFeeExemption(address _beneficiary) public onlyOwner {
750:         // Check that a beneficiary is currently enabled
751:         uint24 hasExemption = uint24(feeOverrides[_beneficiary] & 0xFFFFFF);
752:@>       if (hasExemption != 1) {

In getFee UniswapImplementation.sol#L711-L712

File: UniswapImplementation.sol
698:     function getFee(PoolId _poolId, address _sender) public view returns (uint24 fee_) { 
////code
711:@>       if (uint24(swapFeeOverride & 0xFFFFFF) == 1) {

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

In getfee exemptionfee suppose to override values of poolfee and defaultfee this check will never be true and will never use exemptionfee.

In removeFeeExemption we first check if there is an exemptionfee present which will always return false and will not proccess making the function is useless.

Impact

• We can't override poolfee or defaultfee values.
• we can't use removeFeeExemption function.

PoC

In setFeeExemption we ues this to set the fee feeOverrides[_beneficiary] = uint48(_flatFee) << 24 | 0xFFFFFF; //@audit those bitwise manipulation seems wrong

the output of this operation will be as follow assuming 0 value of _flatFee

   000000000000000000000000000000000000000000000000
   |
   000000000000000000000000111111111111111111111111
   -----------------------------------------------
   000000000000000000000000111111111111111111111111

Then in either getFee or removeFeeExemption goes as follow

   000000000000000000000000111111111111111111111111 (Fee)
   &
   000000000000000000000000111111111111111111111111 (0xFFFFFF)
   -----------------------------------------------
   000000000000000000000000111111111111111111111111

the result is 0xFFFFFF which will never be equal to 1

Mitigation

In removeFeeExemption

-        if (hasExemption != 1) {
+        if (hasExemption != 0xFFFFFF) {

In getFee

-        if (uint24(swapFeeOverride & 0xFFFFFF) == 0xFFFFFF) {
+        if (uint24(swapFeeOverride & 0xFFFFFF) == 0xFFFFFF) {