diff --git a/constraint/solver/hint_registry.go b/constraint/solver/hint_registry.go index 9ea3b573a4..fa5d44d357 100644 --- a/constraint/solver/hint_registry.go +++ b/constraint/solver/hint_registry.go @@ -2,6 +2,7 @@ package solver import ( "fmt" + "github.com/consensys/gnark/internal/hints" "math/big" "sync" @@ -9,7 +10,7 @@ import ( ) func init() { - RegisterHint(InvZeroHint) + RegisterHint(InvZeroHint, hints.Randomize) } var ( diff --git a/frontend/cs/r1cs/api.go b/frontend/cs/r1cs/api.go index a23665fbb8..095ee6e528 100644 --- a/frontend/cs/r1cs/api.go +++ b/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()) diff --git a/internal/hints/hints.go b/internal/hints/hints.go new file mode 100644 index 0000000000..1513706dac --- /dev/null +++ b/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 +} diff --git a/internal/security_tests/advisory-9xcg/advisory_test.go b/internal/security_tests/advisory-9xcg/advisory_test.go new file mode 100644 index 0000000000..fd7488048f --- /dev/null +++ b/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 + } + } +} diff --git a/internal/stats/latest.stats b/internal/stats/latest.stats index dc2f8a08aa..c156102ed0 100644 Binary files a/internal/stats/latest.stats and b/internal/stats/latest.stats differ diff --git a/std/multicommit/nativecommit_test.go b/std/multicommit/nativecommit_test.go index 4b570b7f33..b78f518651 100644 --- a/std/multicommit/nativecommit_test.go +++ b/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)) }