sherlock-audit / 2024-04-titles-judging

1 stars 1 forks source link

alexzoid - Front-Running Vulnerability in `revokeRoles` Function #455

Closed sherlock-admin4 closed 1 month ago

sherlock-admin4 commented 2 months ago

alexzoid

medium

Front-Running Vulnerability in revokeRoles Function

Summary

The revokeRoles function in the TitlesGraph contract could be susceptible to a front-running attack.

Vulnerability Detail

The revokeRoles function is designed to remove roles from a given address (guy). This function checks if the sender (msg.sender) has the ADMIN_ROLE, allowing them to revoke roles from others. However, because transactions are visible in the mempool before being executed, a malicious actor with access to another admin account could potentially observe a legitimate role-revocation transaction and front-run it by either changing the roles or permissions of the target or by performing another sensitive action that depends on the target's roles.

Code Snippet Analysis

https://github.com/sherlock-audit/2024-04-titles/blob/main/wallflower-contract-v2/src/graph/TitlesGraph.sol#L167-L178

/// @notice Override the {OwnableRoles} implementation to extend access to the `ADMIN_ROLE`.
/// @param guy The address from which to revoke the roles.
/// @param roles The roles to revoke.
/// @dev This function is used to revoke roles from an address and will revert if the caller is not the owner and does not have the `ADMIN_ROLE`.
function revokeRoles(address guy, uint256 roles)
    public
    payable
    override
    onlyOwnerOrRoles(ADMIN_ROLE)
{
    _removeRoles(guy, roles);
}

Impact

This can compromise the integrity of the contract's operations.

Tool used

Manual Review

Recommendation

Implement a two-step process for sensitive administrative actions like role changes.