tradi3_ Kurt Landman
Public report archive

Firelight T3-010

Assessment approval can execute a replacement loss schedule the Risk Consortium never reviewed

IncidentManager.approveCurrentAssessment(incidentId) binds only the incident ID in calldata and resolves currentAssessmentRoundId from mutable storage at execution, allowing a cancelled and replaced loss schedule to execute under the Risk Consortium's prior signature.

Severity
High
Outcome
paid
Competition
Audit Comp | Firelight
Vulnerability class
approval binding / access control
Published

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
  1. Round 1 reviewed: Curator submits a 100-unit loss schedule in round 1. Risk Consortium reviews round 1 and signs approveCurrentAssessment(1) (0x62c1f4cb...0001).
  2. Round swapped before execution: Curator calls cancelCurrentAssessment(1), adds a 900-unit loss schedule (incrementing currentAssessmentRoundId to 2), and calls submitCurrentAssessment(1).
  3. Unchanged approval executed: Broadcasting the Consortium’s original signed transaction executes round 2 and slashes 900 units of vault staker principal instead of the reviewed 100 units.

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).

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
}