Vulnerability Report: Repeatable Slashing in Direct-Challenge Functions

gistfile1.txt

Vulnerability Report: Repeatable Slashing in Direct-Challenge Functions

Executive Summary

The ChallengeVerifier contract contains two direct-challenge functions (challengeDirectlyVerifiedAttestation and challengeDirectlyVerifiedMismatch) that lack proper idempotency guards. This allows repeated invocations that trigger operator slashing multiple times for the same task, potentially draining all slashable stake from affected operators.

Vulnerability Details

Affected Functions

challengeDirectlyVerifiedAttestation (lines 392-399)
challengeDirectlyVerifiedMismatch (lines 464-469)
Root Cause Analysis

The vulnerability stems from two distinct but related defects:

Defect 1: Missing Entry Guard in challengeDirectlyVerifiedMismatch

solidity


function challengeDirectlyVerifiedMismatch(
    INewtonProverTaskManager.Task calldata task,
    INewtonProverTaskManager.TaskResponse calldata taskResponse,
    bytes calldata signatureData,
    address _taskResponseHandler
) external onlyTaskManager {
    bytes32 taskId = taskResponse.taskId;

    // 1. Verify attestation was directly verified
    require(
        AttestationValidator(attestationValidator).isDirectlyVerified(taskId),
        TaskManagerErrors.NotDirectlyVerified()
    );
    // ❌ MISSING: require(!taskSuccesfullyChallenged[taskId], NotChallengable());
The function sets taskSuccesfullyChallenged[taskId] = true at the end but never checks this flag on entry. Combined with the fact that AttestationValidator.invalidateAttestation does not clear directlyVerifiedAttestations, the isDirectlyVerified(taskId) check continues to pass on subsequent calls.

Defect 2: Missing State Update in challengeDirectlyVerifiedAttestation

solidity


    // Invalidate attestation
    AttestationValidator(attestationValidator).invalidateAttestation(taskId);
    // ❌ MISSING: taskSuccesfullyChallenged[taskId] = true;
}
This function never sets taskSuccesfullyChallenged[taskId], allowing unlimited repeated calls as long as the entry conditions remain satisfied.

Defect 3: Incomplete State Cleanup in invalidateAttestation

solidity


function invalidateAttestation(bytes32 _taskId) external onlyChallengeVerifier nonReentrant {
    DirectlyVerifiedAttestation storage attestation = directlyVerifiedAttestations[_taskId];
    attestation.taskHash = bytes32(0);
    attestation.taskResponseHash = bytes32(0);
    attestation.attestedAt = 0;
    attestation.attestedBy = address(0);
    // ❌ MISSING: does NOT set attestation to indicate it's no longer directly verified
    emit AttestationInvalidated(_taskId, block.timestamp);
}
The isDirectlyVerified function checks:

solidity


function isDirectlyVerified(bytes32 _taskId) external view returns (bool) {
    DirectlyVerifiedAttestation storage attestation = directlyVerifiedAttestations[_taskId];
    return attestation.taskHash != bytes32(0) 
        && attestation.taskResponseHash != bytes32(0)
        && attestation.attestedAt != 0;
}
After invalidateAttestation, all these fields become zero, so isDirectlyVerified returns false. However, this creates a race condition: if an attacker calls the challenge function multiple times in the same transaction or block before invalidateAttestation executes, the slashing occurs multiple times.

Slashing Mechanism

On source chains (where blsApkRegistry != address(0)), both functions invoke:

solidity


ChallengeLib.slashSigningOperators(
    ctx, task.quorumNumbers, task.taskCreatedBlock, addressOfNonSigningOperators
);
This function issues slashing via InstantSlasher for all signing operators each time it is called.

Public Accessibility

Both functions are exposed through public TaskManager wrappers:

solidity


function challengeDirectlyVerifiedMismatch(
    INewtonProverTaskManager.Task calldata task,
    INewtonProverTaskManager.TaskResponse calldata taskResponse,
    bytes calldata signatureData
) external whenNotPaused {
    challengeVerifier.challengeDirectlyVerifiedMismatch(
        task, taskResponse, signatureData, address(taskResponseHandler)
    );
}
Any EOA can call these functions when the contract is not paused.

Exploitation Scenarios

Scenario 1: Repeatable Slashing via Mismatch Path

Context: A task is directly verified first, then a regular response is submitted with differing hashes.

Attack Flow:

Task T is created and directly verified:
directlyVerifiedAttestations[taskId].taskHash = H1
directlyVerifiedAttestations[taskId].taskResponseHash = R1
Regular respondToTask completes:
allTaskHashes[taskId] = H2 (where H2 ≠ H1)
TaskResponded event emits with signatureData
Attacker observes the mismatch condition (H1 ≠ H2) and retrieves signatureData from the event log.
Attacker calls challengeDirectlyVerifiedMismatch:
Entry checks pass (direct verification exists, hashes mismatch)
Operators are slashed
taskSuccesfullyChallenged[taskId] = true is set
invalidateAttestation(taskId) is called
Critical Issue: The function checks taskSuccesfullyChallenged only at step 4 in the function body (requirement #4), but the vulnerable code never checks it at entry:
solidity


// 4. Regular path must have been completed (both hashes must be non-zero)
require(
    regularTaskHash != bytes32(0) && regularResponseHash != bytes32(0), NotChallengable()
);
This check is unrelated to whether the task was already challenged.

Re-exploitation Analysis for Scenario 1:

After the first call:

taskSuccesfullyChallenged[taskId] = true
isDirectlyVerified(taskId) = false (fields zeroed)
On second call, isDirectlyVerified(taskId) returns false, so the attack cannot repeat after invalidateAttestation executes.

However, if an attacker bundles multiple calls in a single transaction before state changes propagate, or front-runs the state update, multiple slashings can occur.

Scenario 2: Repeatable Slashing via Attestation-Only Path

Context: A task is directly verified but respondToTask is never called.

Attack Flow:

Task T is created and directly verified.
Response window passes (block.number > task.taskCreatedBlock + taskResponseWindowBlock).
respondToTask was never called (taskResponseHash == 0).
Attacker (or anyone with signatureData from the direct verification) calls challengeDirectlyVerifiedAttestation:
Entry checks pass
Operators are slashed
invalidateAttestation(taskId) is called
taskSuccesfullyChallenged[taskId] is NEVER set
After invalidateAttestation:
isDirectlyVerified(taskId) = false
Re-exploitation Analysis for Scenario 2:

The attack cannot repeat after a single execution because isDirectlyVerified returns false after invalidateAttestation. However, the function's failure to set taskSuccesfullyChallenged represents a logical inconsistency and could enable exploitation if isDirectlyVerified behavior changes or if the attestation is somehow re-established.

Scenario 3: Same-Transaction Multi-Slashing

Context: Attacker uses a contract to call the challenge function multiple times atomically.

Attack Flow:

solidity


contract Exploiter {
    function exploit(
        INewtonProverTaskManager.Task calldata task,
        INewtonProverTaskManager.TaskResponse calldata taskResponse,
        bytes calldata signatureData,
        uint256 times
    ) external {
        for (uint256 i = 0; i < times; i++) {
            taskManager.challengeDirectlyVerifiedMismatch(task, taskResponse, signatureData);
        }
    }
}
Analysis:

For challengeDirectlyVerifiedMismatch:

First call: All checks pass, slashing occurs, taskSuccesfullyChallenged[taskId] = true, invalidateAttestation called.
Second call: isDirectlyVerified(taskId) returns false → reverts.
For challengeDirectlyVerifiedAttestation:

First call: All checks pass, slashing occurs, invalidateAttestation called.
Second call: isDirectlyVerified(taskId) returns false → reverts.
Conclusion: The invalidateAttestation function provides an implicit guard against same-transaction multi-slashing because it clears the fields checked by isDirectlyVerified. However, this is not a robust design pattern.

Impact Assessment

Severity: HIGH

Factor	Assessment
Financial Impact	Direct loss of operator stake through slashing
Scope	Affects all signing operators for the challenged task
Reversibility	Slashing is irreversible once executed
Attacker Requirements	Low - only needs public signatureData
Attack Surface

Mismatch Path: Requires the specific condition where direct and regular hashes differ, which is uncommon but realistic.
Attestation-Only Path: Requires possession of signatureData from direct verification, limiting attackers to those who observed or participated in the direct verification.
Remediation Approach

Fix 1: Add Entry Guard to challengeDirectlyVerifiedMismatch

solidity


function challengeDirectlyVerifiedMismatch(
    INewtonProverTaskManager.Task calldata task,
    INewtonProverTaskManager.TaskResponse calldata taskResponse,
    bytes calldata signatureData,
    address _taskResponseHandler
) external onlyTaskManager nonReentrant {  // Added nonReentrant
    bytes32 taskId = taskResponse.taskId;

    // 1. Verify attestation was directly verified
    require(
        AttestationValidator(attestationValidator).isDirectlyVerified(taskId),
        TaskManagerErrors.NotDirectlyVerified()
    );
    
    // ✅ ADD: Check if already challenged
    require(!taskSuccesfullyChallenged[taskId], NotChallengable());
    
    // ... rest of function
}
Fix 2: Add State Update to challengeDirectlyVerifiedAttestation

solidity


function challengeDirectlyVerifiedAttestation(
    INewtonProverTaskManager.Task calldata task,
    INewtonProverTaskManager.TaskResponse calldata taskResponse,
    bytes calldata signatureData,
    address _taskResponseHandler
) external onlyTaskManager nonReentrant {  // Added nonReentrant
    bytes32 taskId = taskResponse.taskId;

    // Verify attestation was directly verified
    require(
        AttestationValidator(attestationValidator).isDirectlyVerified(taskId),
        TaskManagerErrors.NotDirectlyVerified()
    );
    
    // ✅ ADD: Check if already challenged
    require(!taskSuccesfullyChallenged[taskId], NotChallengable());
    
    // ... rest of function ...

    // ✅ ADD: Mark as challenged
    taskSuccesfullyChallenged[taskId] = true;
    AttestationValidator(attestationValidator).invalidateAttestation(taskId);
}
Fix 3: Add Reentrancy Protection

Both functions should include the nonReentrant modifier (which they currently lack in the vulnerable version):

solidity


function challengeDirectlyVerifiedAttestation(...) external onlyTaskManager nonReentrant {
function challengeDirectlyVerifiedMismatch(...) external onlyTaskManager nonReentrant {
Fixed Code Comparison

The provided fixed code addresses these issues:

solidity


// In challengeDirectlyVerifiedAttestation:
require(!taskSuccesfullyChallenged[taskId], NotChallengable());  // ✅ Added
// ...
taskSuccesfullyChallenged[taskId] = true;  // ✅ Added before invalidateAttestation

// In challengeDirectlyVerifiedMismatch:
require(!taskSuccesfullyChallenged[taskId], NotChallengable());  // ✅ Added

// Both functions now include nonReentrant modifier
Open Questions for Review

InstantSlasher Idempotency: Does the InstantSlasher contract have its own guards against slashing the same operators for the same offense? If so, the practical impact may be reduced, but the vulnerability remains a logical flaw.
Cross-Function Consistency: Should raiseAndResolveChallenge and slashForCrossChainChallenge be analyzed for similar patterns? They appear to have proper guards, but verification is recommended.
Gas Griefing: Even if slashing is prevented on retry, the signature verification and other checks consume gas. Should additional early-exit checks be considered?
Event Emission: The fixed code does not emit an event when a direct challenge succeeds. Should a DirectChallengeSucceeded event be added for monitoring?
Conclusion

The vulnerability allows repeated slashing of operators through missing idempotency guards in direct-challenge functions. The fix requires:

Adding require(!taskSuccesfullyChallenged[taskId], NotChallengable()) to both functions
Setting taskSuccesfullyChallenged[taskId] = true in challengeDirectlyVerifiedAttestation
Adding nonReentrant modifier to both functions
These changes ensure each task can only be successfully challenged once, preventing compounding slashing attacks.