Front-run withdraw `_trust()` to grief users

[hats-bug-reporter[bot]]

Github username: -- Twitter username: -- Submission hash (on-chain): 0xec7bc17cdbb6208d02f4a0ac0ea20b9ec3edd1f02deacab6e2ed24110ac2450f Severity: medium

Description:

Impact

Attacker can gas-grief users over and over again and prevent registration and group mint

Description

The root of the problem is that anyone can withdraw their trust and the minimum _expiry is block.timestamp which is a good first step, but not a full mitigation against front-running attacks because if the attacker's transaction is included in an earlier block than the victim's TX (by paying higher gas prices) then the attacker can still successfully front-run users.

Hub.sol - trust()

    function trust(address _trustReceiver, uint96 _expiry) external {
        if (avatars[msg.sender] == address(0)) {
            revert CirclesAvatarMustBeRegistered(msg.sender, 0);
        }
        if (_trustReceiver == address(0) || _trustReceiver == SENTINEL) {
            // You cannot trust the zero address or the sentinel address.
            // Reserved addresses for logic.
            revert CirclesHubInvalidTrustReceiver(_trustReceiver, 1);
        }
        if (_trustReceiver == msg.sender) {
            // You cannot edit your own trust relation.
            revert CirclesHubInvalidTrustReceiver(_trustReceiver, 2);
        }
        // expiring trust cannot be set in the past
        if (_expiry < block.timestamp) _expiry = uint96(block.timestamp);
        _trust(msg.sender, _trustReceiver, _expiry);
    }

Proof of Concept

Registration

1. Attacker invites user via calling trust(user, INDEFINITE_FUTURE)
2. User calls registerHuman() with attacker's invitation
3. Attacker sees user's TX in the mempool and front-runs him with trust(user, block.timestamp) with a higher gas price that ensures attacker's TX is included in a block before the block user's TX will be executed in
4. User's registration will revert since attacker withdrawn his trust and block.timestamp is more in the current block where he called registerHuman()

Group Mint

1. Group trusts collateralAvatar for group mints
2. User wants to mint the group's circles via groupMint()
3. Group monitors mempool and immediately withdraw their trust with trust(collateralAvatar, block.timestamp) with a higher gas price the group's TX is included in a block before the block user's TX will be executed in
4. User's groupMint() will revert since group withdrawn his trust and block.timestamp is more in the current block where he called groupMint()

Recommendation

Instead of setting the _expiry to block.timestamp when the _expiry is less then block.timestamp consider to add more time for the lower bound of expiry to prevent all front-running attacks, not just the ones happening in the same block:

    function trust(address _trustReceiver, uint96 _expiry) external {
        ...
        ...
        // expiring trust cannot be set in the past
-       if (_expiry < block.timestamp) _expiry = uint96(block.timestamp);
+       if (_expiry < block.timestamp - 1 hours) _expiry = uint96(block.timestamp + 1 hours);
        _trust(msg.sender, _trustReceiver, _expiry);
    }

[benjaminbollen]

Thank you for your insightful report on the potential front-running of the _trust() function. We appreciate your attention to detail in identifying this scenario. After careful consideration, we've classified this as a low-priority concern rather than a medium level issue. Here's why:

• The trust system is designed for use between individuals who know and trust each other. The scenario you describe would require intentional malicious action by a trusted party.
• We previously implemented a delay on untrusting to prevent such gas-grievance attacks. However, after thorough evaluation in January, we decided to prioritize user experience and system simplicity over this specific protection.
• While the scenario you describe is technically possible, we believe the social implications of such an attack within trusted relationships outweigh the technical risk.

Your report demonstrates a deep understanding of potential edge cases in blockchain systems. We value such thorough analysis, as it helps us continually evaluate and justify our design decisions. Thank you for contributing to the security and robustness of our platform.

[aktech297]

Thank you for your insightful report on the potential front-running of the _trust() function. We appreciate your attention to detail in identifying this scenario. After careful consideration, we've classified this as a low-priority concern rather than a medium level issue. Here's why:

• The trust system is designed for use between individuals who know and trust each other. The scenario you describe would require intentional malicious action by a trusted party.
• We previously implemented a delay on untrusting to prevent such gas-grievance attacks. However, after thorough evaluation in January, we decided to prioritize user experience and system simplicity over this specific protection.
• While the scenario you describe is technically possible, we believe the social implications of such an attack within trusted relationships outweigh the technical risk.

Your report demonstrates a deep understanding of potential edge cases in blockchain systems. We value such thorough analysis, as it helps us continually evaluate and justify our design decisions. Thank you for contributing to the security and robustness of our platform.

what is the actual problem here ?

how many the attacker is going to spend their gas to invite other and again cancel it by front running. ? In this case, the attacker makes two calls. but the actual user do only one call.

why do user want to use the attacker address as inviter ? They can use the reliable address as inviter and proceed the registration.

imo, this issue is over inflated one.

[benjaminbollen]

I'm going to copy paste my own original arguments, because I made a mistake marking this as a 'low' issue at the beginning. It clearly is not an issue and @aktech297 only put a finer point on it.

• The trust system is designed for use between individuals who know and trust each other. The scenario you describe would require intentional malicious action by a trusted party.
• We previously implemented a delay on untrusting to prevent such gas-grievance attacks. However, after thorough evaluation in January, we decided to prioritize user experience and system simplicity over this specific protection.
• While the scenario you describe is technically possible, we believe the social implications of such an attack within trusted relationships outweigh the technical risk.

Apologies for wrongly labeling this initially as 'low', but I think all readers will agree this is not an issue.

[0xfuje]

Responding to @aktech297:

what is the actual problem here ?

It's described in detail above.

how many the attacker is going to spend their gas to invite other and again cancel it by front running. ? In this case, the attacker makes two calls. but the actual user do only one call.

The attacker making two calls does not mean they have to spend more gas then the user in one call. registerHuman is also a much more gas intensive function compared to trust due to first registering themsg.sender and burning / minting of tokens (updating total supply, etc). Even if they would have to spend equal or more gas that would still be a valid attack.

why do user want to use the attacker address as inviter ? They can use the reliable address as inviter and proceed the registration.

Because only the attacker invited them? This is a permissioned system, someone has to invite users who want to register, they can't just use any address.

[0xfuje]

Appreciate the thoughtful and fast judging work @benjaminbollen!

Here's why I believe this is a valid attack: Even though this is a permissioned system, we can't assume a trusted party will always act non-maliciously. I believe we shouldn't rely on social implications to mitigate such attacks. It's rare but there are griefing and gas-griefing attacks on-chain with the sole goal of griefing (without profit).

Technically an attacker could invite multiple (10+ or more) people to Circles only to grief them (without any investment beside the gas costs) potentially repeating this attack as long as they try to re-register which would be damaging in gas-costs for users, denying them of their expected registration plus passive damage to the project's reputation. I think the tiny additional code that adds some time to the minimum expiry time is worthwhile to implement if you want to make sure to prevent this type of attack.

[aktech297]

Appreciate the thoughtful and fast judging work @benjaminbollen!

Here's why I believe this is a valid attack: Even though this is a permissioned system, we can't assume a trusted party will always act non-maliciously. I believe we shouldn't rely on social implications to mitigate such attacks. It's rare but there are griefing and gas-griefing attacks on-chain with the sole goal of griefing (without profit).

Technically an attacker could invite multiple (10+ or more) people to Circles only to grief them (without any investment beside the gas costs) potentially repeating this attack as long as they try to re-register which would be damaging in gas-costs for users, denying them of their expected registration plus passive damage to the project's reputation. I think the tiny additional code that adds some time to the minimum expiry time is worthwhile to implement if you want to make sure to prevent this type of attack.

Hey.. why do you think that user want to use attacker invitation?? Why not somebody else invite which is reasonable.

[0xfuje]

Hey.. why do you think that user want to use attacker invitation?? Why not somebody else invite which is reasonable.

Because they are expecting to be registered with the attacker's invitation, they can't know in advance trust() will be withdrawn and their transaction will fail. With a single invitation they can't choose another inviter, only if multiple people invite them.

[aktech297]

Hey.. why do you think that user want to use attacker invitation?? Why not somebody else invite which is reasonable.

Because they are expecting to be registered with the attacker's invitation, they can't know in advance trust() will be withdrawn and their transaction will fail. With a single invitation they can't choose another inviter, only if multiple people invite them.

To me .. it's trust issue..

[benjaminbollen]

Because only the attacker invited them? This is a permissioned system, someone has to invite users who want to register, they can't just use any address.

this is a common mistake throughout many issues raised: the invitation is not here to make it a permissioned system, it only serves to encourage wallets to onboard new users as connected to the main trust graph so that the graph remains connected and people's Circles are usable.

[benjaminbollen]

also worth mentioning that older implementations had a delay on the untrust, but we decided to simplify this and get rid of it. see eg https://github.com/aboutcircles/circles-contracts-v2/blob/chiado-v0.1.0/src/graph/Graph.sol#L738-L741