code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-03-aragon/blob/main/packages/contracts/src/core/permission/PermissionManager.sol#L320

Vulnerability details

Impact

In the __isGranted function, if an exception is thrown during the execution of the permission condition contract's isGranted function, the program will jump to the catch block, and the function will return false. This prevents the exception from propagating to the calling contract and potentially causing unexpected behavior.

Proof of Concept

311:        try
            IPermissionCondition(accessFlagOrCondition).isGranted(
                _where,
                _who,
                _permissionId,
                _data
            )
        returns (bool allowed) {
            if (allowed) return true;
        } catch {}

Tools Used

VS Code

Recommended Mitigation Steps

In the provided function, if an exception is caught during the execution of the permission condition contract's isGranted function, the function returns false. While this prevents the exception from propagating to the calling contract, it does not provide any information about the cause of the error.

To provide more informative error handling, the catch block could be modified to include an error message or error code that provides more detail about the cause of the exception. For example, the catch block could emit an event that includes an error code and a message describing the issue.

0xean commented 1 year ago

This would break the return, but either way is a QA level concern and warden doesn't show needed impact for a M sev.

c4-judge commented 1 year ago

0xean changed the severity to QA (Quality Assurance)