ctf_sec - Owner check logical should use && instead of || when rebalancing

[sherlock-admin2]

ctf_sec

medium

Owner check logical should use && instead of || when rebalancing

Summary

Owner logical should use && instead of ||

Vulnerability Detail

function rebalance(
        address payable _srcOft,
        uint16 _dstChainId,
        uint256 _slippage,
        uint256 _amount,
        bytes memory _ercData
    ) external payable onlyValidDestination(_srcOft, _dstChainId) onlyValidSlippage(_slippage) {

        // @audit
        // is this correct? use && instead of
        if (msg.sender != owner() || msg.sender != rebalancer) revert NotAuthorized();

Impact

In the contract, Balancer.sol

we should check

both owner() and rebalancer

if (msg.sender != owner() && msg.sender != rebalancer) revert NotAuthorized();

otherwise, balancer address wants to trigger rebalance, but balancer != owner() is True, transaction will revert in NotAuthorized();

Impact

.

Code Snippet

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

Tool used

Manual Review

Recommendation

use && instead of ||

Duplicate of #89

[maarcweiss]

Dup of https://github.com/sherlock-audit/2024-02-tapioca-judging/issues/52

[sherlock-admin4]

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

WangAudit commented:

I would say the issue is valid but the report doesn't mention the impact and if the rebalancer and owner can be different addresses. Therefore; this is invalid cause it doesn't properly explain what's the issue