Skip to content

Conversation

@Oppen
Copy link
Contributor

@Oppen Oppen commented Jan 17, 2025

An operator could trigger a nil-dereference in the aggregator and bring it down by sending a message with a nil-signature.
Catch this error, log a warning and return an error instead.

The log is level warning to avoid polluting with unnecessary stack traces.

Fixes #1747

Type of change

  • Bug fix

Checklist

  • “Hotfix” to testnet, everything else to staging
  • Linked to Github Issue
  • This change depends on code or research by an external entity
    • Acknowledgements were updated to give credit
  • Unit tests added
  • This change requires new documentation.
    • Documentation has been added/updated.
  • This change is an Optimization
    • Benchmarks added/run
  • Has a known issue
  • If your PR changes the Operator compatibility (Ex: Upgrade prover versions)
    • This PR adds compatibility for operator for both versions and do not change batcher/docs/examples
    • This PR updates batcher and docs/examples to the newer version. This requires the operator are already updated to be compatible

Testing

Apply the following patch:

diff --git a/operator/pkg/rpc_client.go b/operator/pkg/rpc_client.go
index 5726f14d..414f9c86 100644
--- a/operator/pkg/rpc_client.go
+++ b/operator/pkg/rpc_client.go
@@ -38,6 +38,10 @@ func NewAggregatorRpcClient(aggregatorIpPortAddr string, logger logging.Logger)
 // their signed task response.
 func (c *AggregatorRpcClient) SendSignedTaskResponseToAggregator(signedTaskResponse *types.SignedTaskResponse) {
 	var reply uint8
+
+	// @audit nil derefernce
+	signedTaskResponse.BlsSignature.G1Point = nil
+
 	for retries := 0; retries < MaxRetries; retries++ {
 		err := c.rpcClient.Call("Aggregator.ProcessOperatorSignedTaskResponseV2", signedTaskResponse, &reply)
 		if err != nil {

Then start sending proofs, the aggregator should log an error an keep going. In testnet this should crash the aggregator.

@Oppen Oppen added audit cantina Audit report from Cantina labels Jan 20, 2025
@MauroToscano MauroToscano merged commit ea6188f into testnet Jan 28, 2025
3 checks passed
@MauroToscano MauroToscano deleted the hotfix/aggregator_nil_deref branch January 28, 2025 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

audit cantina Audit report from Cantina

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: malformed operator response can crash the aggregator

5 participants