feat add random mask to groth16 commitment (#1245)

* feat add random mask to groth16 commitment

* fix register rand hint

* update latest.stats

* test: add regression test

* test: fail if witness found

* test: multicommit with zero commitments

* test: multicommit with all backends

---------

Co-authored-by: Arya Tabaie <[email protected]>
Co-authored-by: Ivo Kubjas <[email protected]>

constraint/solver/hint_registry.go

@@ -2,14 +2,15 @@ package solver
 
 import (
 	"fmt"
+	"github.com/consensys/gnark/internal/hints"
 	"math/big"
 	"sync"
 
 	"github.com/consensys/gnark/logger"
 )
 
 func init() {
-	RegisterHint(InvZeroHint)
+	RegisterHint(InvZeroHint, hints.Randomize)
 }
 
 var (

frontend/cs/r1cs/api.go

@@ -19,6 +19,7 @@ package r1cs
 import (
 	"errors"
 	"fmt"
+	"github.com/consensys/gnark/internal/hints"
 	"path/filepath"
 	"reflect"
 	"runtime"
@@ -687,6 +688,19 @@ func (builder *builder) Compiler() frontend.Compiler {
 
 func (builder *builder) Commit(v ...frontend.Variable) (frontend.Variable, error) {
 
+	// add a random mask to v
+	{
+		vCp := make([]frontend.Variable, len(v)+1)
+		copy(vCp, v)
+		mask, err := builder.NewHint(hints.Randomize, 1)
+		if err != nil {
+			return nil, err
+		}
+		vCp[len(v)] = mask[0]
+		builder.cs.AddR1C(builder.newR1C(mask[0], builder.eOne, mask[0]), builder.genericGate) // the variable needs to be involved in a constraint otherwise it will not affect the commitment
+		v = vCp
+	}
+
 	commitments := builder.cs.GetCommitments().(constraint.Groth16Commitments)
 	existingCommitmentIndexes := commitments.CommitmentIndexes()
 	privateCommittedSeeker := utils.MultiListSeeker(commitments.GetPrivateCommitted())

internal/hints/hints.go

@@ -0,0 +1,20 @@
+package hints
+
+import (
+	"crypto/rand"
+	"errors"
+	"math/big"
+)
+
+func Randomize(mod *big.Int, ins, outs []*big.Int) error {
+	if len(ins) != 0 {
+		return errors.New("randomize takes no input")
+	}
+	var err error
+	for i := range outs {
+		if outs[i], err = rand.Int(rand.Reader, mod); err != nil {
+			return err
+		}
+	}
+	return nil
+}

internal/security_tests/advisory-9xcg/advisory_test.go

@@ -0,0 +1,89 @@
+// Package advisory9xcg implements a test for advisory GHSA-9xcg-3q8v-7fq6.
+package advisory9xcg
+
+import (
+	"crypto/rand"
+	"fmt"
+	"math/big"
+	"testing"
+
+	"github.com/consensys/gnark-crypto/ecc"
+	"github.com/consensys/gnark-crypto/ecc/bn254"
+	"github.com/consensys/gnark/backend/groth16"
+	groth16_bn254 "github.com/consensys/gnark/backend/groth16/bn254"
+	"github.com/consensys/gnark/frontend"
+	"github.com/consensys/gnark/frontend/cs/r1cs"
+	"github.com/consensys/gnark/test"
+)
+
+type Circuit struct {
+	SecretWitness frontend.Variable `gnark:",private"`
+}
+
+func (circuit *Circuit) Define(api frontend.API) error {
+	// the goal of the test is to show that we are able to predict the private
+	// input solely from the stored commitment.
+	commitCompiler, ok := api.Compiler().(frontend.Committer)
+	if !ok {
+		return fmt.Errorf("compiler does not commit")
+	}
+
+	commit, err := commitCompiler.Commit(circuit.SecretWitness)
+	if err != nil {
+		return err
+	}
+
+	api.AssertIsDifferent(commit, 0)
+	api.AssertIsDifferent(circuit.SecretWitness, 0)
+	return nil
+}
+
+func TestAdvisory_ghsa_9xcg_3q8v_7fq6(t *testing.T) {
+	assert := test.NewAssert(t)
+	// the goal of the test is to show that we are able to predict the private
+	// input solely from the stored commitment
+
+	// Generating a random secret witness.
+	var bound int64 = 1024 // ten bits of entropy for testing
+	secretWitness, err := rand.Int(rand.Reader, big.NewInt(bound))
+	assert.NoError(err, "random generation failed")
+	assert.Log("random secret witness: ", secretWitness)
+
+	// Assigning some values.
+	assignment := Circuit{SecretWitness: secretWitness}
+	witness, err := frontend.NewWitness(&assignment, ecc.BN254.ScalarField())
+	assert.NoError(err, "witness creation failed")
+	witnessPublic, err := witness.Public()
+	assert.NoError(err, "witness public failed")
+
+	// Setup circuit
+	ccs, err := frontend.Compile(ecc.BN254.ScalarField(), r1cs.NewBuilder, &Circuit{})
+	assert.NoError(err, "compilation failed")
+
+	// run the setup and prover
+	pk, vk, err := groth16.Setup(ccs)
+	assert.NoError(err, "setup failed")
+	proof, err := groth16.Prove(ccs, pk, witness)
+	assert.NoError(err, "proof failed")
+
+	// sanity check, check that the proof verifies
+	err = groth16.Verify(proof, vk, witnessPublic)
+	assert.NoError(err, "verification failed")
+
+	// we're ready to set up the attack. For that first we need to assert the
+	// exact types for being able to extract the proving key information.
+	pkConcrete, ok := pk.(*groth16_bn254.ProvingKey)
+	assert.True(ok, "unexpected type for proving key")
+	proofConcrete, ok := proof.(*groth16_bn254.Proof)
+	assert.True(ok, "unexpected type for proof")
+
+	var guessedCommitment bn254.G1Affine
+	for i := int64(0); i < bound; i++ {
+		// We check our guess for the secret witness.
+		guessedCommitment.ScalarMultiplication(&pkConcrete.CommitmentKeys[0].Basis[0], big.NewInt(int64(i)))
+		if guessedCommitment.Equal(&proofConcrete.Commitments[0]) {
+			assert.Fail("secret witness found: ", i)
+			return
+		}
+	}
+}

std/multicommit/nativecommit_test.go

@@ -4,7 +4,6 @@ import (
 	"testing"
 
 	"github.com/consensys/gnark-crypto/ecc"
-	"github.com/consensys/gnark/backend"
 	"github.com/consensys/gnark/frontend"
 	"github.com/consensys/gnark/frontend/cs/r1cs"
 	"github.com/consensys/gnark/test"
@@ -52,7 +51,7 @@ func TestMultipleCommitments(t *testing.T) {
 	circuit := multipleCommitmentCircuit{}
 	assignment := multipleCommitmentCircuit{X: 10}
 	assert := test.NewAssert(t)
-	assert.ProverSucceeded(&circuit, &assignment, test.WithCurves(ecc.BN254), test.WithBackends(backend.GROTH16)) // right now PLONK doesn't implement commitment
+	assert.ProverSucceeded(&circuit, &assignment, test.WithCurves(ecc.BN254)) // right now PLONK doesn't implement commitment
 }
 
 type noCommitVariable struct {
@@ -64,9 +63,11 @@ func (c *noCommitVariable) Define(api frontend.API) error {
 	return nil
 }
 
+// TestNoCommitVariable checks that a circuit that doesn't use the commitment variable
+// compiles and prover succeeds. This is due to the randomization of the commitment.
 func TestNoCommitVariable(t *testing.T) {
 	circuit := noCommitVariable{}
+	assignment := noCommitVariable{X: 10}
 	assert := test.NewAssert(t)
-	_, err := frontend.Compile(ecc.BN254.ScalarField(), r1cs.NewBuilder, &circuit)
-	assert.Error(err)
+	assert.ProverSucceeded(&circuit, &assignment, test.WithCurves(ecc.BN254))
 }