It is not possible to allow a public permission access to a function while removing the permission from another function in the same target

[code423n4]

Lines of code

https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/common/AllowList.sol#L46

Vulnerability details

Impact

It is not possible to block functionX and give public access to functionY, in the same target.

Proof of Concept

Suppose there is a vulnerability in the protocol, and zkSync decides to prevent any user from withdrawing from zkSync while depositing is allowed. But, with the current structure of AllowList contract, it is only possible to make all the functions of a target public access permission or just give permission of a function to a user.

https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/common/AllowList.sol#L79

In summary, it is not possible to give public access to the function deposit, while limiting the access of the function finalizeWithdrawal in L1ERC20Bridge.

Tools Used

Recommended Mitigation Steps

Following mapping is needed: target => function signatures that are excluded

mapping(address => mapping(bytes4 => bool)) public excludedFunction;

The following function should be modified:

function canCall(
        address _caller,
        address _target,
        bytes4 _functionSig
    ) external view returns (bool) {
        return
            (isAccessPublic[_target] ||
                hasSpecialAccessToCall[_caller][_target][_functionSig]) &
            !excludedFunction[_target][_functionSig];
    }

[GalloDaSballo]

Looks like a refactoring in the best case

[c4-sponsor]

miladpiri marked the issue as disagree with severity

[miladpiri]

It can not lead to security issue. Because most probably if there is a bug in a function of a facet, all the funactions of the facet should be blocked (for sake of security), untill it is upgraded through diamond cut. So, this is a QA issue!

[c4-sponsor]

miladpiri marked the issue as sponsor acknowledged

[GalloDaSballo]

Downgrading to Refactoring

[GalloDaSballo]

R

[c4-judge]

Duplicate of https://github.com/code-423n4/2022-10-zksync-findings/issues/49

[miladpiri]

Duplicate of #49

IMO, it is not duplicate of #49.