PermissionChecker Manual Review Findings

PCR-01M: Discrepant XOR Behaviour

Type	Severity	Location
Logical Fault		PermissionChecker.sol:L431

Description:

The logical XOR operator implemented by the PermissionChecker deviates from how the XOR operation would behave if applied to each individual child of the condition. In detail, a XOR sequence of 1 XOR 1 XOR 1 would yield a final output of 1, however, a PermissionChecker::_xor operation of such child conditions would yield a final output of 0.

Impact:

Conditions that are meant to utilize the XOR operator would behave differently than the traditional logical XOR operator, potentially causing users to misuse it.

Example:

Copy
406function _xor(
407    uint256 value,
408    bytes calldata data,
409    Condition memory condition,
410    ParameterPayload memory payload,
411    Consumption[] memory consumptions
412) private pure returns (Status, Result memory result) {
413    uint256 okCount;
414    unchecked {
415        for (uint256 i; i < condition.children.length; ++i) {
416            (Status status, Result memory _result) = _walk(
417                value,
418                data,
419                condition.children[i],
420                payload,
421                consumptions
422            );
423            if (status == Status.Ok) {
424                result = _result;
425                okCount = okCount + 1;
426            }
427        }
428    }
429
430    return
431        okCount == 1
432            ? (Status.Ok, result)
433            : (
434                Status.XorViolation,
435                Result({consumptions: consumptions, info: 0})
436            );
437}

Recommendation:

We advise this behaviour to be clearly documented in the PermissionChecker and if the deviation is undesirable, the referenced conditional to apply a modulo operation (%) to okCount with 2 imitating the behaviour of sequential XOR operations as in a real logical circuit.

Alleviation:

The Gnosis Guild team evaluated this exhibit, accepted that the present XOR behaviour is discrepant, and ultimately opted to remove this type of functionality from the logical subset of operators entirely. As such, we consider this exhibit alleviated.