code-423n4 / 2024-10-ronin-findings

0 stars 0 forks source link

Expired Accounts Continue To be Authorized #63

Closed howlbot-integration[bot] closed 4 weeks ago

howlbot-integration[bot] commented 4 weeks ago

Lines of code

https://github.com/ronin-chain/katana-operation-contracts/blob/27f9d28e00958bf3494fa405a8a5acdcd5ecdc5d/src/governance/KatanaGovernance.sol#L378

Vulnerability details

Proof of Concept

KatanaGovernance may specify a configurable expiry data for authorized accounts, however due to an invalid comparator, expired authorizations remain active for longer than intended:

/**
 * @dev Checks if an account is authorized.
 * @param account The address of the account to check authorization for.
 * @return A boolean indicating whether the account is authorized or not.
 */
function _isAuthorized(Permission storage $, address account) private view returns (bool) {
    uint256 expiry = $.whitelistUntil;
    if (expiry == UNAUTHORIZED) return false;
@>  if (expiry == AUTHORIZED || block.timestamp > expiry) return true;

    return $.allowed[account];
}

Notice here that if the expiry is neither configured to UNAUTHORIZED nor AUTHORIZED, it will be compared against the block.timestamp:

if (expiry == AUTHORIZED || block.timestamp > expiry) return true;

Here, we see that if the block.timestamp exceeds the configured expiry, it will continue to be interpreted as authorized.

Recommended Mitigation Steps

Invert the comparison:

- if (expiry == AUTHORIZED || block.timestamp > expiry) return true;
+ if (expiry == AUTHORIZED || block.timestamp < expiry) return true;

Assessed type

Invalid Validation

c4-judge commented 3 weeks ago

alex-ppg marked the issue as unsatisfactory: Invalid