code-423n4 / 2024-07-optimism-findings

3 stars 0 forks source link

The lack of early reverts in `FDG.move` and `FDG.step` functions makes the protocol more centralized #17

Open c4-bot-7 opened 4 months ago

c4-bot-7 commented 4 months ago

Lines of code

https://github.com/code-423n4/2024-07-optimism/blob/70556044e5e080930f686c4e5acde420104bb2c4/packages/contracts-bedrock/src/dispute/FaultDisputeGame.sol#L307 https://github.com/code-423n4/2024-07-optimism/blob/70556044e5e080930f686c4e5acde420104bb2c4/packages/contracts-bedrock/src/dispute/FaultDisputeGame.sol#L391

Vulnerability details

Honest challengers making moves and steps against invalid claims is the cornerstone of the fault proofs system. It is safe to assume that all honest challengers will likely take the same actions at the same time since these actions are valid for honest participants and they are incentivized to act as quickly as possible.

This leads to a situation where honest challengers compete with each other to call the move and step functions. It is also clear that only the honest challenger who wins this race will be able to claim the bond. All other honest challengers will lose money due to gas expenses. The problem with the current implementation is that it doesn't attempt to reduce gas costs for challengers who lose the race. Let's consider the move and step functions separately below.

The move function implements the check that this particular move was already made as a last check.

if (claims[claimHash]) revert ClaimAlreadyExists();

It means that honest challengers who lose the race will be obligated to spend significantly more gas than they should. This issue inevitably disincentivizes honest challengers from participating in the protocol.

The step function checks that the step was already made almost at the end of the function after executing VM.step.

if (parent.counteredBy != address(0)) revert DuplicateStep();

It means that honest challengers who lose the race are obligated to spend almost the full amount of gas without any compensation. The issue becomes even more severe because this function can be front-run by sophisticated MEV bots without the necessity to run challenger software. This makes the competition even more intense. This inevitably decreases the probability that a challenger will win and thus disincentivizes honest challengers from calling this function at all.

I believe this problem can't be solved entirely, but we still should do everything possible to reduce the costs of running challenger software. The profitability of challenges directly affects the decentralization of the system, and the decentralization of the system directly affects its safety due to:

Impact

The lack of early reverts in the move and step functions disincentivizes honest challengers from participating in the protocol, leading to the centralization of the protocol and making whale attacks significantly cheaper. I also want to highlight that this issue cannot be solved in the challenger software and is therefore in scope.

Proof of Concept

-

Tools Used

Manual Review

Recommended Mitigation Steps

Consider moving the aforementioned checks as close to the beginning of the functions as possible.

Assessed type

Invalid Validation

zobront commented 4 months ago

This is valid but I'm not sure if it justifies a Medium. Will ask for sponsor to weigh in with their perspective.

ajsutton commented 4 months ago

Agreed this is accurate but very low impact. The overwhelming majority of costs to run an honest challenger is in keeping it running to monitor the games even if there's nothing invalid to counter. The potential gas cost savings here are unlikely to make any actual difference to whether someone is willing to run a challenger or not.

c4-judge commented 4 months ago

zobront changed the severity to QA (Quality Assurance)

c4-judge commented 4 months ago

zobront marked the issue as grade-a

zobront commented 4 months ago

I'm in agreement with the sponsor here. This is good advice but not a Medium severity issue. Downgrading to QA.