Skip to content

Commit 6ddd627

Browse files
authored
fix: task replay directed at same operator set (#1629)
**Motivation:** Fixing a bug where a random actor could permissionlessly call `TaskMailbox::submitResult` with an old cert+result combination (as long as the cert is not stale) but for a completely different task directed at the same operator set in the TaskMailbox. This could lead to DoS of the taskCreator. **Modifications:** The `messageHash` of the cert now is `keccak256(abi.encode(taskHash, result))` instead of just `keccak256(result)`. This ensures that the messageHash is applicable only for a specific task and can't be replayed across tasks. **Result:** Fixes `SigmaPrime: ELHG-02` finding.
1 parent d12b92a commit 6ddd627

File tree

3 files changed

+161
-56
lines changed

3 files changed

+161
-56
lines changed

src/contracts/avs/task/TaskMailbox.sol

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ contract TaskMailbox is
208208
task.executorOperatorSetTaskConfig.consensus,
209209
executorOperatorSet,
210210
task.operatorTableReferenceTimestamp,
211-
keccak256(result),
211+
getMessageHash(taskHash, result),
212212
executorCert
213213
);
214214

@@ -406,31 +406,31 @@ contract TaskMailbox is
406406
* @notice Validates a BN254 certificate's basic requirements
407407
* @param cert The BN254 certificate to validate
408408
* @param operatorTableReferenceTimestamp The expected reference timestamp
409-
* @param resultHash The expected message hash
409+
* @param messageHash The expected message hash
410410
*/
411411
function _validateBN254Certificate(
412412
IBN254CertificateVerifierTypes.BN254Certificate memory cert,
413413
uint32 operatorTableReferenceTimestamp,
414-
bytes32 resultHash
414+
bytes32 messageHash
415415
) internal pure {
416416
require(cert.referenceTimestamp == operatorTableReferenceTimestamp, InvalidReferenceTimestamp());
417-
require(cert.messageHash == resultHash, InvalidMessageHash());
417+
require(cert.messageHash == messageHash, InvalidMessageHash());
418418
require(!(cert.signature.X == 0 && cert.signature.Y == 0), EmptyCertificateSignature());
419419
}
420420

421421
/**
422422
* @notice Validates an ECDSA certificate's basic requirements
423423
* @param cert The ECDSA certificate to validate
424424
* @param operatorTableReferenceTimestamp The expected reference timestamp
425-
* @param resultHash The expected message hash
425+
* @param messageHash The expected message hash
426426
*/
427427
function _validateECDSACertificate(
428428
IECDSACertificateVerifierTypes.ECDSACertificate memory cert,
429429
uint32 operatorTableReferenceTimestamp,
430-
bytes32 resultHash
430+
bytes32 messageHash
431431
) internal pure {
432432
require(cert.referenceTimestamp == operatorTableReferenceTimestamp, InvalidReferenceTimestamp());
433-
require(cert.messageHash == resultHash, InvalidMessageHash());
433+
require(cert.messageHash == messageHash, InvalidMessageHash());
434434
require(cert.sig.length > 0, EmptyCertificateSignature());
435435
}
436436

@@ -440,15 +440,15 @@ contract TaskMailbox is
440440
* @param consensus The consensus configuration
441441
* @param executorOperatorSet The executor operator set
442442
* @param operatorTableReferenceTimestamp The reference timestamp of the operator table
443-
* @param resultHash The hash of the result of the task
443+
* @param messageHash The hash of the message that was signed by the operators
444444
* @param executorCert The executor certificate to verify
445445
*/
446446
function _verifyExecutorCertificate(
447447
IKeyRegistrarTypes.CurveType curveType,
448448
Consensus memory consensus,
449449
OperatorSet memory executorOperatorSet,
450450
uint32 operatorTableReferenceTimestamp,
451-
bytes32 resultHash,
451+
bytes32 messageHash,
452452
bytes memory executorCert
453453
) internal {
454454
if (consensus.consensusType == ConsensusType.NONE) {
@@ -462,7 +462,7 @@ contract TaskMailbox is
462462
abi.decode(executorCert, (IBN254CertificateVerifierTypes.BN254Certificate));
463463

464464
// Validate the certificate
465-
_validateBN254Certificate(bn254Cert, operatorTableReferenceTimestamp, resultHash);
465+
_validateBN254Certificate(bn254Cert, operatorTableReferenceTimestamp, messageHash);
466466

467467
IBN254CertificateVerifier(BN254_CERTIFICATE_VERIFIER).verifyCertificate(executorOperatorSet, bn254Cert);
468468
} else if (curveType == IKeyRegistrarTypes.CurveType.ECDSA) {
@@ -471,7 +471,7 @@ contract TaskMailbox is
471471
abi.decode(executorCert, (IECDSACertificateVerifierTypes.ECDSACertificate));
472472

473473
// Validate the certificate
474-
_validateECDSACertificate(ecdsaCert, operatorTableReferenceTimestamp, resultHash);
474+
_validateECDSACertificate(ecdsaCert, operatorTableReferenceTimestamp, messageHash);
475475

476476
IECDSACertificateVerifier(ECDSA_CERTIFICATE_VERIFIER).verifyCertificate(executorOperatorSet, ecdsaCert);
477477
} else {
@@ -493,7 +493,7 @@ contract TaskMailbox is
493493
abi.decode(executorCert, (IBN254CertificateVerifierTypes.BN254Certificate));
494494

495495
// Validate the certificate
496-
_validateBN254Certificate(bn254Cert, operatorTableReferenceTimestamp, resultHash);
496+
_validateBN254Certificate(bn254Cert, operatorTableReferenceTimestamp, messageHash);
497497

498498
isThresholdMet = IBN254CertificateVerifier(BN254_CERTIFICATE_VERIFIER).verifyCertificateProportion(
499499
executorOperatorSet, bn254Cert, totalStakeProportionThresholds
@@ -504,7 +504,7 @@ contract TaskMailbox is
504504
abi.decode(executorCert, (IECDSACertificateVerifierTypes.ECDSACertificate));
505505

506506
// Validate the certificate
507-
_validateECDSACertificate(ecdsaCert, operatorTableReferenceTimestamp, resultHash);
507+
_validateECDSACertificate(ecdsaCert, operatorTableReferenceTimestamp, messageHash);
508508

509509
(isThresholdMet,) = IECDSACertificateVerifier(ECDSA_CERTIFICATE_VERIFIER).verifyCertificateProportion(
510510
executorOperatorSet, ecdsaCert, totalStakeProportionThresholds
@@ -586,4 +586,9 @@ contract TaskMailbox is
586586
) external pure returns (bytes memory) {
587587
return abi.encode(cert);
588588
}
589+
590+
/// @inheritdoc ITaskMailbox
591+
function getMessageHash(bytes32 taskHash, bytes memory result) public pure returns (bytes32) {
592+
return keccak256(abi.encode(taskHash, result));
593+
}
589594
}

src/contracts/interfaces/ITaskMailbox.sol

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -427,6 +427,14 @@ interface ITaskMailbox is ITaskMailboxErrors, ITaskMailboxEvents {
427427
IECDSACertificateVerifierTypes.ECDSACertificate memory cert
428428
) external pure returns (bytes memory);
429429

430+
/**
431+
* @notice Gets the message hash for a task that needs to be signed by operators
432+
* @param taskHash Unique identifier of the task
433+
* @param result Task execution result data
434+
* @return The message hash for the task that needs to be signed by operators
435+
*/
436+
function getMessageHash(bytes32 taskHash, bytes memory result) external pure returns (bytes32);
437+
430438
/**
431439
* @notice Gets the current fee split percentage
432440
* @return The fee split in basis points

0 commit comments

Comments
 (0)