Consensys / gnark-crypto

gnark-crypto provides elliptic curve and pairing-based cryptography on BN, BLS12, BLS24 and BW6 curves. It also provides various algorithms (algebra, crypto) of particular interest to zero knowledge proof systems.
Apache License 2.0
495 stars 160 forks source link

Possible MiMC bug #234

Closed niconiconi closed 1 year ago

niconiconi commented 2 years ago

In MiMC implementation, I found following code in ecc/bn254/fr/mimc/mimc.go:

const (

    mimcNbRounds = 91

    seed         = "seed"   // seed to derive the constants

    BlockSize    = fr.Bytes // BlockSize size that mimc consumes

)

,where fr.Bytes = 32.

It will be problematic here:

    for i := 0; i < nbChunks; i++ {
        copy(buffer[:], d.data[i*BlockSize:(i+1)*BlockSize])
        x.SetBytes(buffer[:])
        r := d.encrypt(x)
        d.h.Add(&r, &d.h).Add(&d.h, &x)
    }

The buffer can be viewed as a 256-bit Integer.

MiMC users can input any data, so it's possible that buffer >= Fr.p. So x.SetBytes(buffer[:]) may overflow.

niconiconi commented 2 years ago

the overflow can still be secure, but it needs some analysis