From 9d9281372b475a362c0eb3d6739612d8411b151d Mon Sep 17 00:00:00 2001 From: Mario Rugiero Date: Mon, 20 Jan 2025 14:14:55 -0300 Subject: [PATCH] hotfix: workaround gnark 0.10.0 unsoundness bug A lot of time passed since gnark v0.10.0 unsoundness bug[0] was reported and fixed. We posponed the upgrade because the fixed release, v0.11.0, contains another vulnerability, an OOM[1], for which a fix has been in main since last November but no release appeared until now. Our options here are limited, and none quite happy: - We can `redirect` to a commit in `main`; - We can disable groth16 verifiers from the network, which we currently use; or - We can enforce that proofs have only one commitment, as the unsoundness can only be triggered with multiple commitments per proof. This implements the latter option, being the least invasive one. [0]: https://www.zellic.io/blog/gnark-bug-groth16-commitments [1]: https://github.com/Consensys/gnark/security/advisories/GHSA-cph5-3pgr-c82g --- batcher/aligned-batcher/gnark/verifier.go | 12 ++++++++++++ operator/pkg/operator.go | 13 +++++++++++++ 2 files changed, 25 insertions(+) diff --git a/batcher/aligned-batcher/gnark/verifier.go b/batcher/aligned-batcher/gnark/verifier.go index 7e4fe547f..c8e61d173 100644 --- a/batcher/aligned-batcher/gnark/verifier.go +++ b/batcher/aligned-batcher/gnark/verifier.go @@ -20,6 +20,7 @@ import ( "github.com/consensys/gnark-crypto/ecc" "github.com/consensys/gnark/backend/groth16" + bn254 "github.com/consensys/gnark/backend/groth16/bn254" "github.com/consensys/gnark/backend/plonk" "github.com/consensys/gnark/backend/witness" ) @@ -98,6 +99,17 @@ func verifyGroth16Proof(proofBytesRef C.ListRef, pubInputBytesRef C.ListRef, ver return false } + bn254Proof, ok := proof.(*bn254.Proof) + if !ok { + log.Printf("groth16 proof is not bn254") + return false + } + numCommitments := len(bn254Proof.Commitments) + if numCommitments > 1 { + log.Printf("too many commitments (%v) for groth16 proof (unsound for v0.10.0)", numCommitments) + return false + } + pubInputReader := bytes.NewReader(pubInputBytes) pubInput, err := witness.New(curve.ScalarField()) if err != nil { diff --git a/operator/pkg/operator.go b/operator/pkg/operator.go index d4c36355c..8e6fcbc95 100644 --- a/operator/pkg/operator.go +++ b/operator/pkg/operator.go @@ -31,6 +31,7 @@ import ( eigentypes "github.com/Layr-Labs/eigensdk-go/types" "github.com/consensys/gnark-crypto/ecc" "github.com/consensys/gnark/backend/groth16" + bn254 "github.com/consensys/gnark/backend/groth16/bn254" "github.com/consensys/gnark/backend/plonk" "github.com/consensys/gnark/backend/witness" ethcommon "github.com/ethereum/go-ethereum/common" @@ -598,6 +599,18 @@ func (o *Operator) verifyGroth16Proof(proofBytes []byte, pubInputBytes []byte, v return false } + bn254Proof, ok := proof.(*bn254.Proof) + if !ok { + o.Logger.Warn("groth16 proof is not bn254") + return false + } + numCommitments := len(bn254Proof.Commitments) + if numCommitments > 1 { + o.Logger.Warn("too many commitments for groth16 proof (unsound for v0.10.0)", + "numCommitments", numCommitments) + return false + } + pubInputReader := bytes.NewReader(pubInputBytes) pubInput, err := witness.New(curve.ScalarField()) if err != nil {