Summary
Published Immunefi report #88618.
In Firelight’s incident payout architecture, CURATOR_ROLE proposes an incident loss schedule and the independent Risk Consortium (ASSESSMENT_APPROVER_ROLE) reviews that schedule and signs approveCurrentAssessment(uint256 incidentId).
Because approveCurrentAssessment takes only incidentId and does not bind the assessment round ID or schedule hash in calldata, the contract resolves incident.currentAssessmentRoundId from storage when the transaction executes. After the Risk Consortium signs an approval for round N (for example, a reviewed 100-unit payout), the curator can call cancelCurrentAssessment(incidentId), submit round N + 1 with a 900-unit loss schedule, and broadcast the Consortium’s unchanged signed approval transaction to execute the unreviewed 900-unit payout.
Root cause
In contracts/core/IncidentManager.sol (v1_audit_ready @ 42f9ea5e43d88b35197b901acc2a8c24b314fa62), approveCurrentAssessment accepts only incidentId:
function approveCurrentAssessment(uint256 incidentId)
external
onlyRole(ASSESSMENT_APPROVER_ROLE)
nonReentrant
{
Incident storage incident =
_activeIncidentWithStatus(incidentId, IncidentStatus.UNDER_EVALUATION);
// ... FIFO check ...
(uint256 assessmentRoundId, AssessmentRound storage assessmentRound) =
_setCurrentAssessmentStatus(
incidentId,
incident,
AssessmentRoundStatus.UNDER_EVALUATION,
AssessmentRoundStatus.APPROVED
);
_executePayout(/* ... */, assessmentRound.totalAssessmentLoss);
}
Inside _setCurrentAssessmentStatus(), the round is read dynamically from storage:
assessmentRoundId = incident.currentAssessmentRoundId;
assessmentRound = $.assessmentRounds[incidentId][assessmentRoundId];
When ASSESSMENT_APPROVER_ROLE is a multisig (such as Safe v1.4.1), the signers sign keccak256(calldata) where calldata is abi.encodeWithSelector(approveCurrentAssessment.selector, 1). Between signature collection and on-chain execution, CURATOR_ROLE can cancel round 1, populate round 2 with a different loss schedule, and call submitCurrentAssessment(1), leaving incident. status == UNDER_EVALUATION and currentAssessmentRoundId == 2.
Proof of concept
Running npx hardhat test test/PoC_AssessmentRoundSwap.js test/incidentManager/IncidentManager.test.js --no-compile (PoC gist):
reviewed payout: 100000000
executed replacement payout: 900000000
real vault staker assets: 1000000000 -> 100000000
unreviewed excess slash over 100-unit schedule: 800000000
107 passing
- Round 1 reviewed: Curator submits a 100-unit loss schedule in round 1. Risk Consortium reviews round 1 and signs
approveCurrentAssessment(1)(0x62c1f4cb...0001). - Round swapped before execution: Curator calls
cancelCurrentAssessment(1), adds a 900-unit loss schedule (incrementingcurrentAssessmentRoundIdto2), and callssubmitCurrentAssessment(1). - Unchanged approval executed: Broadcasting the Consortium’s original signed transaction executes round 2 and slashes
900units of vault staker principal instead of the reviewed100units.
Impact
Bypasses the core separation-of-duties invariant between the incident curator and the independent Risk Consortium, allowing an unreviewed replacement loss schedule to drain vault staker principal (1,000 -> 100 in the production-vault test).
Recommended fix
Bind approveCurrentAssessment and rejectCurrentAssessment to the exact expectedAssessmentRoundId (or schedule hash) reviewed by the approver:
function approveCurrentAssessment(
uint256 incidentId,
uint256 expectedAssessmentRoundId
) external onlyRole(ASSESSMENT_APPROVER_ROLE) nonReentrant {
Incident storage incident = _activeIncidentWithStatus(
incidentId,
IncidentStatus.UNDER_EVALUATION
);
if (incident.currentAssessmentRoundId != expectedAssessmentRoundId) {
revert AssessmentRoundChanged(
expectedAssessmentRoundId,
incident.currentAssessmentRoundId
);
}
// proceed with approval and payout
}