Using OR operator instead of AND operator in rebalance() will make call always fail if owner() ≠ rebalancer

Summary

Wrong usage of OR operator instead of AND operator when checking the caller in reabalance() will make the call always fail.

Vulnerability Detail

The rebalance() function in Balancer.sol should only be triggered by Balancer's owner or by the rebalancer address.

From Tapioca’s contest documentation regarding off-chain mechanisms, the rebalancer address will be a Gelato bot.

The problem is that the rebalance() function uses an OR operator to check the msg.sender, instead of an AND operator. This will make the rebalance() function always fail if owner() and rebalancer are different addresses.

// Balancer.sol

function rebalance(
        address payable _srcOft,
        uint16 _dstChainId,
        uint256 _slippage,
        uint256 _amount,
        bytes memory _ercData
    ) external payable onlyValidDestination(_srcOft, _dstChainId) onlyValidSlippage(_slippage) {
        if (msg.sender != owner() || msg.sender != rebalancer) revert NotAuthorized();  
        ...

        }

Although rebalancer and the contract’s owner are set to the same address on initialization, it is unlikely that the contract’s owner and the rebalancer itself will be the same address in production,(it wouldn’t make sense to check for both addresses if they are expected to be the same). Also, most functions in Balancer.sol are onlyOwner protected, and these type of functions such as initConnectedOFT() or emergencySaveTokens() won’t be automated and handled by the Gelato bot.

Impact

High. The function will be uncallable and will always revert with NotAuthorized().

Code Snippet

https://github.com/sherlock-audit/2024-02-tapioca/blob/main/TapiocaZ/contracts/Balancer.sol#L176

Tool used

Manual Review

Recommendation

Apply the following change in rebalance():

// Balancer.sol

function rebalance(
        address payable _srcOft,
        uint16 _dstChainId,
        uint256 _slippage,
        uint256 _amount,
        bytes memory _ercData
    ) external payable onlyValidDestination(_srcOft, _dstChainId) onlyValidSlippage(_slippage) {
-        if (msg.sender != owner() || msg.sender != rebalancer) revert NotAuthorized();  
+       if (msg.sender != owner() && msg.sender != rebalancer) revert NotAuthorized();  
     ...
 }

Duplicate of #89

sherlock-audit / 2024-02-tapioca-judging

0xadrii - Using OR operator instead of AND operator in rebalance() will make call always fail if owner() ≠ rebalancer #123