Attacking an agreed upon counter claim further to the left leads to unexpected loss of bonds
Low/Info issue submitted by jovi
Summary
In order for claims be resolved, they must be dangling - that is, don't have a a pending subgame attached to themselves.
The game resolves to true the deepest and leftmost counter claim that has no valid subgames, sends the bond to the claimant or the counterer and from that starting point will climb back up the claims/counter claims tree in order to distribute the bonds and resolve the game.
The issue is: correct counter claims can be freely attacked and lose their bonds if their grandparents/grandchildren are not at the step call family.
Vulnerability Detail
As the last dangling counter claim is the winner, it must receive a bond payment for that proper behavior. What happens in practice is the counter claim that lead to the step is utilized to pay for it, according to the following code snippet:
function step(
...
parent.counteredBy = msg.sender;
}
This brings a series of consequences to the previous counter claims at the resolveClaim level, as now the claim (we'll call it #08 to fit the PoC) that lead to the step was countered. So the claim #07 that lead to #08 will enter the following portion of the resolveClaim function and not be countered by anyone, as its subgame claim (#08) had been previously countered at the step() call. The claimant for claim #08 is left empty-handed, even though its counter claim lead to the correct result.
At the end, we can see that #07 is countered by address(0).
if (claim.counteredBy == address(0) && leftmostCounter.raw() > claim.position.raw()) {
countered = claim.claimant;
leftmostCounter = claim.position;
}
}
// If the parent was not successfully countered, pay out the parent's bond to the claimant.
// If the parent was successfully countered, pay out the parent's bond to the challenger.
_distributeBond(countered == address(0) ? parent.claimant : countered, parent);
parent.counteredBy = countered;
Now, when we call resolveClaim to #06, the caller of the attack that lead to claim #07 will receive the bond, as #07 counteredBy variable is set to address(0) in the previous call.
When we resolveClaim to #05, #06 claimant is left empty-handed as it can't be set as the counterer for the current claim as it was countered by #07 claimant.
The following POC displays how all claims attack up until they reach a leftmost node at the maximum depth by stepping on it. Then the stepper actor claims the last bond and finally it displays that even though some actors had correctly counter claimed, they have lost their bonds.
Paste the following code snippet at the FaultDisputeGame.t.sol file:
function test_poc_d_03() public {
// Give the test contract some ether
uint256 bal = 1000 ether;
vm.deal(address(this), bal);
address[8] memory addresses = [
address(0x1),
address(0x2),
address(0x3),
address(0x4),
address(0x5),
address(0x6),
address(0x7),
address(0x8)
];
// Loop to deal ether to each address
for (uint i = 0; i < addresses.length; i++) {
vm.deal(addresses[i], 100 ether);
}
// Initialize totalBonded to keep track of total bonds
uint256 totalBonded = 0;
// Loop through each address to simulate attacks
for (uint i = 0; i < addresses.length; i++) {
uint256 bond = _getRequiredBond(i);
totalBonded += bond;
vm.prank(addresses[i]);
if (i == 4) {
gameProxy.attack{ value: bond }(i, _changeClaimStatus(_dummyClaim(), VMStatuses.PANIC));
} else {
gameProxy.attack{ value: bond }(i, _dummyClaim());
}
}
gameProxy.addLocalData(LocalPreimageKey.DISPUTED_L2_BLOCK_NUMBER, 8, 0);
gameProxy.step(8, true, absolutePrestateData, hex"");
// Resolve all claims
vm.warp(block.timestamp + 3 days + 12 hours + 1 seconds);
for (uint256 i = gameProxy.claimDataLen(); i > 0; i--) {
(bool success,) = address(gameProxy).call(abi.encodeCall(gameProxy.resolveClaim, (i - 1)));
assertTrue(success);
}
gameProxy.resolve();
// Wait for the withdrawal delay.
vm.warp(block.timestamp + delayedWeth.delay() + 1 seconds);
// Loop to claim credit, handling special cases
// Notice half the counter claimers actually lost their bonds
// Even though their counter claims pushed it to the correct outcome
for (uint256 i = 1; i < 8; i++) {
if (i == 2 || i == 4 || i == 6 || i == 8) {
vm.expectRevert(NoCreditToClaim.selector);
gameProxy.claimCredit(addresses[i - 1]);
} else {
gameProxy.claimCredit(addresses[i - 1]);
}
}
}
Run the tests with the following command:
forge test --match-test test_poc_d_03 -vvvvv
Impact
Even though the counter claim that lead to the step is rightfully proven wrong, the same cannot be said about the counter claims #02, #04 and #06 as all counter claims have opted to attack.
This means the system can be exploited by malicious parties by attacking correct counter claims up until the step function in order to earn their bonds.
Parties will be incentivized to never allow a game clock's time to run out at the hash bisection phase as this means exposure to losing their bonds.
Users may be afraid to place counter claims as they may be a losing game even though their claims are correct.
Users will be incentivized to front-run counter claims that belong to max depth's family (be it odd or even) and to avoid doing counter claims at the other family.
The bonding reward logic currently does not handle the edge case in which parties agree but still opt into going deeper into the game anyway. As it can be maliciously done to steal correct counter claim's bonds, the bond distribution logic should be reworked to handle such case.
There's no easy fix to this as the system is very complex.
A suggestion would be to check whether the counter claim actually represents a behavior similar to the claim it has countered and not reward it. This stops those redundant counter claims from being rewarded by attacking an attack movement it already agrees with.
Attacking an agreed upon counter claim further to the left leads to unexpected loss of bonds
Low/Info issue submitted by jovi
Summary
In order for claims be resolved, they must be dangling - that is, don't have a a pending subgame attached to themselves. The game resolves to true the deepest and leftmost counter claim that has no valid subgames, sends the bond to the claimant or the counterer and from that starting point will climb back up the claims/counter claims tree in order to distribute the bonds and resolve the game. The issue is: correct counter claims can be freely attacked and lose their bonds if their grandparents/grandchildren are not at the step call family.
Vulnerability Detail
As the last dangling counter claim is the winner, it must receive a bond payment for that proper behavior. What happens in practice is the counter claim that lead to the step is utilized to pay for it, according to the following code snippet:
This brings a series of consequences to the previous counter claims at the resolveClaim level, as now the claim (we'll call it #08 to fit the PoC) that lead to the step was countered. So the claim #07 that lead to #08 will enter the following portion of the resolveClaim function and not be countered by anyone, as its subgame claim (#08) had been previously countered at the step() call. The claimant for claim #08 is left empty-handed, even though its counter claim lead to the correct result. At the end, we can see that #07 is countered by address(0).
Now, when we call resolveClaim to #06, the caller of the attack that lead to claim #07 will receive the bond, as #07 counteredBy variable is set to address(0) in the previous call. When we resolveClaim to #05, #06 claimant is left empty-handed as it can't be set as the counterer for the current claim as it was countered by #07 claimant.
The following POC displays how all claims attack up until they reach a leftmost node at the maximum depth by stepping on it. Then the stepper actor claims the last bond and finally it displays that even though some actors had correctly counter claimed, they have lost their bonds. Paste the following code snippet at the FaultDisputeGame.t.sol file:
Run the tests with the following command:
Impact
Even though the counter claim that lead to the step is rightfully proven wrong, the same cannot be said about the counter claims #02, #04 and #06 as all counter claims have opted to attack. This means the system can be exploited by malicious parties by attacking correct counter claims up until the step function in order to earn their bonds. Parties will be incentivized to never allow a game clock's time to run out at the hash bisection phase as this means exposure to losing their bonds. Users may be afraid to place counter claims as they may be a losing game even though their claims are correct. Users will be incentivized to front-run counter claims that belong to max depth's family (be it odd or even) and to avoid doing counter claims at the other family.
Code Snippet
https://github.com/ethereum-optimism/optimism/blob/develop/packages/contracts-bedrock/src/dispute/FaultDisputeGame.sol#L219 https://github.com/ethereum-optimism/optimism/blob/develop/packages/contracts-bedrock/src/dispute/FaultDisputeGame.sol#L464C8-L468C40
Tool used
Manual Review
Recommendation
The bonding reward logic currently does not handle the edge case in which parties agree but still opt into going deeper into the game anyway. As it can be maliciously done to steal correct counter claim's bonds, the bond distribution logic should be reworked to handle such case. There's no easy fix to this as the system is very complex. A suggestion would be to check whether the counter claim actually represents a behavior similar to the claim it has countered and not reward it. This stops those redundant counter claims from being rewarded by attacking an attack movement it already agrees with.