Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug: Rangechecker.Check passes even when bits int is 1 less than bit-length of v variable in case when bits is odd #897

Closed
ultrainstinct30 opened this issue Oct 31, 2023 · 1 comment
Assignees
Labels
bug Something isn't working P2: Medium Issue priority: medium

Comments

@ultrainstinct30
Copy link

Description

After creating a new instance of Rangechecker by using rangecheck.New(api), calling Check function using this instance passes even when the input bits is 1 less than the actual bit-length of v when bits is odd.

Expected Behavior

Check should fail

Actual Behavior

Check passes, implying v has bit-length bits

Steps to Reproduce

import (
	"testing"

	"github.com/consensys/gnark-crypto/ecc"
	"github.com/consensys/gnark/frontend"
	"github.com/consensys/gnark/frontend/cs/r1cs"
	"github.com/consensys/gnark/std/rangecheck"
	"github.com/consensys/gnark/test"
)

type TestRangeCheckCircuit struct {
	I1 frontend.Variable
	N  int
}

func (circuit *TestRangeCheckCircuit) Define(api frontend.API) error {
	rangeChecker := rangecheck.New(api)
	rangeChecker.Check(circuit.I1, circuit.N)
	return nil
}

func TestRangeCheck_Odd(t *testing.T) {
	assert := test.NewAssert(t)

	type testData struct {
		i1 uint64
		n  int
	}

	tests := []testData{}
	for i := 128; i < 256; i++ { //[128, 256) all have bit-length 8
		tests = append(tests, testData{i1: uint64(i), n: 7})
	}

	for _, t_i := range tests {
		circuit := TestRangeCheckCircuit{
			N: t_i.n,
		}
		r1cs, err := frontend.Compile(ecc.BN254.ScalarField(), r1cs.NewBuilder, &circuit)
		if err != nil {
			t.Fatal("Error in compiling circuit: ", err)
		}
		var witness TestRangeCheckCircuit
		witness.I1 = t_i.i1
		w, err := frontend.NewWitness(&witness, ecc.BN254.ScalarField())
		if err != nil {
			t.Fatal("Error in witness: ", err, "\n test: ", t_i)
		}
		err = r1cs.IsSolved(w)
		if err != nil {
			t.Fatal("Circuit not solved: ", err, "\n test: ", t_i)
		}

		assert.CheckCircuit(&circuit, test.WithValidAssignment(&witness), test.WithCurves(ecc.BN254))
	}
}

func TestRangeCheck_Even(t *testing.T) {
	assert := test.NewAssert(t)

	type testData struct {
		i1 uint64
		n  int
	}

	tests := []testData{}
	for i := 64; i < 128; i++ { //[64, 128) all have bit-length 7
		tests = append(tests, testData{i1: uint64(i), n: 6})
	}

	for _, t_i := range tests {
		circuit := TestRangeCheckCircuit{
			N: t_i.n,
		}
		r1cs, err := frontend.Compile(ecc.BN254.ScalarField(), r1cs.NewBuilder, &circuit)
		if err != nil {
			t.Fatal("Error in compiling circuit: ", err)
		}
		var witness TestRangeCheckCircuit
		witness.I1 = t_i.i1
		w, err := frontend.NewWitness(&witness, ecc.BN254.ScalarField())
		if err != nil {
			t.Fatal("Error in witness: ", err, "\n test: ", t_i)
		}
		err = r1cs.IsSolved(w)
		if err != nil {
			t.Fatal("Circuit not solved: ", err, "\n test: ", t_i)
		}

		assert.CheckCircuit(&circuit, test.WithValidAssignment(&witness), test.WithCurves(ecc.BN254))
	}
}

TestRangeCheck_Odd output: ok
TestRangeCheck_Even output: Circuit not solved: constraint #0 is not satisfied: 1 ⋅ 0 != 64 test: {64 6}

Your Environment

  • gnark version used: v0.9.1
  • gnark-crypto version used: v0.12.2-0.20231013160410-1f65e75b6dfb
  • go version: go1.21.3 linux/amd64
  • Operating System and version: Ubuntu 20.04 WSL 2
@ivokub ivokub self-assigned this Oct 31, 2023
@ivokub ivokub added bug Something isn't working P2: Medium Issue priority: medium labels Oct 31, 2023
@ivokub
Copy link
Collaborator

ivokub commented Oct 31, 2023

Thank you for the report. I will have a look at it promptly.

@ivokub ivokub closed this as completed Nov 10, 2023
roman-khimov added a commit to nspcc-dev/neo-go that referenced this issue Aug 27, 2024
Fixes a security issue, Consensys/gnark#897

Signed-off-by: Roman Khimov <roman@nspcc.ru>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working P2: Medium Issue priority: medium
Projects
None yet
Development

No branches or pull requests

2 participants