Skip to content

Commit 4cf83fa

Browse files
committed
fix: submitResult certificate checks (#1557)
**Motivation:** We need to add some additional certificate checks in the `submitResult` function of the TaskMailbox to prevent malicious behavior. **Modifications:** * Require check that the referenceTimestamp of the certificate equals the referenceTimestamp snapshotted at task creation time. * Require check that the messageHash of the certificate matches the hash of the task result. **Result:** Safe `submitResult`.
1 parent 66078af commit 4cf83fa

File tree

8 files changed

+782
-570
lines changed

8 files changed

+782
-570
lines changed

pkg/bindings/ITaskMailbox/binding.go

Lines changed: 197 additions & 112 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/bindings/TaskMailbox/binding.go

Lines changed: 35 additions & 165 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/bindings/TaskMailboxStorage/binding.go

Lines changed: 73 additions & 182 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/contracts/avs/task/TaskMailbox.sol

Lines changed: 43 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,10 @@ contract TaskMailbox is
134134
// Calculate the AVS fee using the task hook
135135
uint96 avsFee = taskConfig.taskHook.calculateTaskFee(taskParams);
136136

137+
// Get the operator table reference timestamp
138+
uint32 operatorTableReferenceTimestamp = IBaseCertificateVerifier(_getCertificateVerifier(taskConfig.curveType))
139+
.latestReferenceTimestamp(taskParams.executorOperatorSet);
140+
137141
bytes32 taskHash = keccak256(abi.encode(_globalTaskCount, address(this), block.chainid, taskParams));
138142
_globalTaskCount = _globalTaskCount + 1;
139143

@@ -147,6 +151,7 @@ contract TaskMailbox is
147151
feeSplit,
148152
TaskStatus.CREATED,
149153
false, // isFeeRefunded
154+
operatorTableReferenceTimestamp,
150155
taskConfig,
151156
taskParams.payload,
152157
bytes(""),
@@ -168,6 +173,7 @@ contract TaskMailbox is
168173
taskHash,
169174
taskParams.executorOperatorSet.avs,
170175
taskParams.executorOperatorSet.id,
176+
operatorTableReferenceTimestamp,
171177
taskParams.refundCollector,
172178
avsFee,
173179
block.timestamp + taskConfig.taskSLA,
@@ -194,6 +200,8 @@ contract TaskMailbox is
194200
task.executorOperatorSetTaskConfig.curveType,
195201
task.executorOperatorSetTaskConfig.consensus,
196202
executorOperatorSet,
203+
task.operatorTableReferenceTimestamp,
204+
keccak256(result),
197205
executorCert
198206
);
199207
require(isCertificateValid, CertificateVerificationFailed());
@@ -320,6 +328,23 @@ contract TaskMailbox is
320328
emit ExecutorOperatorSetRegistered(msg.sender, operatorSet.avs, operatorSet.id, isRegistered);
321329
}
322330

331+
/**
332+
* @notice Gets the certificate verifier for a given curve type
333+
* @param curveType The curve type to get the certificate verifier for
334+
* @return The address of the certificate verifier
335+
*/
336+
function _getCertificateVerifier(
337+
IKeyRegistrarTypes.CurveType curveType
338+
) internal view returns (address) {
339+
if (curveType == IKeyRegistrarTypes.CurveType.BN254) {
340+
return BN254_CERTIFICATE_VERIFIER;
341+
} else if (curveType == IKeyRegistrarTypes.CurveType.ECDSA) {
342+
return ECDSA_CERTIFICATE_VERIFIER;
343+
} else {
344+
revert InvalidCurveType();
345+
}
346+
}
347+
323348
/**
324349
* @notice Validates that the caller is the owner of the operator set
325350
* @param operatorSet The operator set to validate ownership for
@@ -329,14 +354,7 @@ contract TaskMailbox is
329354
OperatorSet memory operatorSet,
330355
IKeyRegistrarTypes.CurveType curveType
331356
) internal view {
332-
address certificateVerifier;
333-
if (curveType == IKeyRegistrarTypes.CurveType.BN254) {
334-
certificateVerifier = BN254_CERTIFICATE_VERIFIER;
335-
} else if (curveType == IKeyRegistrarTypes.CurveType.ECDSA) {
336-
certificateVerifier = ECDSA_CERTIFICATE_VERIFIER;
337-
} else {
338-
revert InvalidCurveType();
339-
}
357+
address certificateVerifier = _getCertificateVerifier(curveType);
340358

341359
require(
342360
IBaseCertificateVerifier(certificateVerifier).getOperatorSetOwner(operatorSet) == msg.sender,
@@ -366,13 +384,17 @@ contract TaskMailbox is
366384
* @param curveType The curve type used for signature verification
367385
* @param consensus The consensus configuration
368386
* @param executorOperatorSet The executor operator set
387+
* @param operatorTableReferenceTimestamp The reference timestamp of the operator table
388+
* @param resultHash The hash of the result of the task
369389
* @param executorCert The executor certificate to verify
370390
* @return isCertificateValid Whether the certificate is valid
371391
*/
372392
function _verifyExecutorCertificate(
373393
IKeyRegistrarTypes.CurveType curveType,
374394
Consensus memory consensus,
375395
OperatorSet memory executorOperatorSet,
396+
uint32 operatorTableReferenceTimestamp,
397+
bytes32 resultHash,
376398
bytes memory executorCert
377399
) internal returns (bool isCertificateValid) {
378400
if (consensus.consensusType == ConsensusType.STAKE_PROPORTION_THRESHOLD) {
@@ -387,7 +409,9 @@ contract TaskMailbox is
387409
IBN254CertificateVerifierTypes.BN254Certificate memory bn254Cert =
388410
abi.decode(executorCert, (IBN254CertificateVerifierTypes.BN254Certificate));
389411

390-
// Validate that the certificate has a non-empty signature
412+
// Validate the certificate
413+
require(bn254Cert.referenceTimestamp == operatorTableReferenceTimestamp, InvalidReferenceTimestamp());
414+
require(bn254Cert.messageHash == resultHash, InvalidMessageHash());
391415
require(bn254Cert.signature.X != 0 && bn254Cert.signature.Y != 0, EmptyCertificateSignature());
392416

393417
isCertificateValid = IBN254CertificateVerifier(BN254_CERTIFICATE_VERIFIER).verifyCertificateProportion(
@@ -398,7 +422,15 @@ contract TaskMailbox is
398422
IECDSACertificateVerifierTypes.ECDSACertificate memory ecdsaCert =
399423
abi.decode(executorCert, (IECDSACertificateVerifierTypes.ECDSACertificate));
400424

401-
// Validate that the certificate has a non-empty signature
425+
// Validate the certificate
426+
require(ecdsaCert.referenceTimestamp == operatorTableReferenceTimestamp, InvalidReferenceTimestamp());
427+
require(
428+
ecdsaCert.messageHash
429+
== IECDSACertificateVerifier(ECDSA_CERTIFICATE_VERIFIER).calculateCertificateDigest(
430+
ecdsaCert.referenceTimestamp, resultHash
431+
),
432+
InvalidMessageHash()
433+
);
402434
require(ecdsaCert.sig.length > 0, EmptyCertificateSignature());
403435

404436
(isCertificateValid,) = IECDSACertificateVerifier(ECDSA_CERTIFICATE_VERIFIER)
@@ -439,6 +471,7 @@ contract TaskMailbox is
439471
task.feeSplit,
440472
_getTaskStatus(task),
441473
task.isFeeRefunded,
474+
task.operatorTableReferenceTimestamp,
442475
task.executorOperatorSetTaskConfig,
443476
task.payload,
444477
task.executorCert,

src/contracts/interfaces/ITaskMailbox.sol

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,12 +102,13 @@ interface ITaskMailboxTypes {
102102
// Storage slot 2: avs (20 bytes) + avsFee (12 bytes) = 32 bytes
103103
address avs;
104104
uint96 avsFee;
105-
// Storage slot 3: refundCollector (20 bytes) + executorOperatorSetId (4 bytes) + feeSplit (2 bytes) + status (1 byte) + isFeeRefunded (1 byte) = 28 bytes (4 bytes unused)
105+
// Storage slot 3: refundCollector (20 bytes) + executorOperatorSetId (4 bytes) + feeSplit (2 bytes) + status (1 byte) + isFeeRefunded (1 byte) + referenceTimestamp (4 bytes) = 32 bytes
106106
address refundCollector;
107107
uint32 executorOperatorSetId;
108108
uint16 feeSplit;
109109
TaskStatus status;
110110
bool isFeeRefunded;
111+
uint32 operatorTableReferenceTimestamp;
111112
// Dynamic storage slots
112113
ExecutorOperatorSetTaskConfig executorOperatorSetTaskConfig;
113114
bytes payload;
@@ -162,6 +163,9 @@ interface ITaskMailboxErrors is ITaskMailboxTypes {
162163
/// @notice Thrown when an invalid consensus value is provided
163164
error InvalidConsensusValue();
164165

166+
/// @notice Thrown when a certificate has an invalid reference timestamp
167+
error InvalidReferenceTimestamp();
168+
165169
/// @notice Thrown when a certificate has an empty signature
166170
error EmptyCertificateSignature();
167171

@@ -176,6 +180,9 @@ interface ITaskMailboxErrors is ITaskMailboxTypes {
176180

177181
/// @notice Thrown when fee split value is invalid (> 10000 bips)
178182
error InvalidFeeSplit();
183+
184+
/// @notice Thrown when a certificate has an invalid message hash
185+
error InvalidMessageHash();
179186
}
180187

181188
/**
@@ -214,6 +221,7 @@ interface ITaskMailboxEvents is ITaskMailboxTypes {
214221
* @param taskHash Unique identifier of the task
215222
* @param avs Address of the AVS handling the task
216223
* @param executorOperatorSetId ID of the executor operator set
224+
* @param operatorTableReferenceTimestamp Reference timestamp of the operator table
217225
* @param refundCollector Address to receive refunds
218226
* @param avsFee Fee paid to the AVS
219227
* @param taskDeadline Timestamp by which the task must be completed
@@ -224,6 +232,7 @@ interface ITaskMailboxEvents is ITaskMailboxTypes {
224232
bytes32 indexed taskHash,
225233
address indexed avs,
226234
uint32 executorOperatorSetId,
235+
uint32 operatorTableReferenceTimestamp,
227236
address refundCollector,
228237
uint96 avsFee,
229238
uint256 taskDeadline,

src/test/mocks/MockECDSACertificateVerifier.sol

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,8 +103,8 @@ contract MockECDSACertificateVerifier is IECDSACertificateVerifier {
103103
return bytes32(0);
104104
}
105105

106-
function calculateCertificateDigest(uint32, /*referenceTimestamp*/ bytes32 /*messageHash*/ ) external pure returns (bytes32) {
107-
return bytes32(0);
106+
function calculateCertificateDigest(uint32 referenceTimestamp, bytes32 messageHash) external pure returns (bytes32) {
107+
return keccak256(abi.encode(referenceTimestamp, messageHash));
108108
}
109109

110110
function getTotalStakeWeights(OperatorSet calldata, uint32) external pure returns (uint[] memory) {

src/test/mocks/MockECDSACertificateVerifierFailure.sol

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,8 +103,8 @@ contract MockECDSACertificateVerifierFailure is IECDSACertificateVerifier {
103103
return bytes32(0);
104104
}
105105

106-
function calculateCertificateDigest(uint32, /*referenceTimestamp*/ bytes32 /*messageHash*/ ) external pure returns (bytes32) {
107-
return bytes32(0);
106+
function calculateCertificateDigest(uint32 referenceTimestamp, bytes32 messageHash) external pure returns (bytes32) {
107+
return keccak256(abi.encode(referenceTimestamp, messageHash));
108108
}
109109

110110
function getTotalStakeWeights(OperatorSet calldata, uint32) external pure returns (uint[] memory) {

0 commit comments

Comments
 (0)