Permission Revocation Handling

[c4-bot-7]

Lines of code

https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/policies/Stop.sol#L299

Vulnerability details

The contract does not handle the case where permissions are revoked while a rental stop is in progress. If the kernel revokes permissions, the contract's calls to dependent modules may fail.

// Interaction: Remove rentals from storage by computing the order hash.
STORE.removeRentals(
    _deriveRentalOrderHash(order),
    _convertToStatic(rentalAssetUpdates)
);

Mitigation

Implement a mechanism to check if the contract still has the necessary permissions before making calls to dependent modules. This could involve querying the kernel for the current permissions before executing critical actions.

// Check if the contract has permission to call removeRentals on STORE
function _hasPermissionToRemoveRentals() private view returns (bool) {
    // Assume kernel exposes a function to check permissions
    return kernel.hasPermission(address(this), address(STORE), STORE.removeRentals.selector);
}

// Use the permission check in the stopRent function
function stopRent(RentalOrder calldata order) external {
    // ... existing code ...
    if (!_hasPermissionToRemoveRentals()) {
        revert("Permission to remove rentals revoked");
    }
    STORE.removeRentals(
        _deriveRentalOrderHash(order),
        _convertToStatic(rentalAssetUpdates)
    );
    // ... rest of the function ...
}

Impact

If permissions are revoked mid-execution, subsequent calls to dependent modules will fail, potentially leaving the contract in an inconsistent state. By checking permissions before making these calls, we can ensure that the contract only attempts actions it is authorized to perform, thus maintaining consistency and preventing failed transactions due to permission issues. This also provides a clearer error message to users, helping them understand why a transaction failed.

Assessed type

Error

[c4-pre-sort]

141345 marked the issue as insufficient quality report

[c4-judge]

0xean marked the issue as unsatisfactory: Insufficient quality