Scalar from random issue

[shargon]

When we create a Scalar object from Random data, it doesn't have the same input validation as FromBytes, this seems like a bug, (but I'm not sure because the input it's different).

https://github.com/neo-project/Neo.Cryptography.BLS12_381/blob/844bc3a4f7d8ba2c545ace90ca124f8ada4c8d29/src/Neo.Cryptography.BLS12_381/Scalar.cs#L44

https://github.com/neo-project/Neo.Cryptography.BLS12_381/blob/844bc3a4f7d8ba2c545ace90ca124f8ada4c8d29/src/Neo.Cryptography.BLS12_381/Scalar.cs#L60-L74

Note: The same happens with FpX

[cschuchardt88]

Please check the Unit Test They all checkout.

[Jim8y]

please check out the rust implementation here: https://github.com/zkcrypto/bls12_381/blob/4df45188913e9d66ef36ae12825865347eed4e1b/src/scalar.rs#L300

[shargon]

please check out the rust implementation here: https://github.com/zkcrypto/bls12_381/blob/4df45188913e9d66ef36ae12825865347eed4e1b/src/scalar.rs#L300

But if you initialize from bytes and you have a restriction, if you init from random bytes, you should have the same restriction, isn't it?

[shargon]

class MyRand : RandomNumberGenerator
{
    public static readonly byte[] INVALID = new byte[]
    {
        1, 0, 0, 0, 255, 255, 255, 255, 254, 91, 254, 255, 2, 164, 189, 83, 5, 216, 161, 9, 8,
        216, 57, 51, 72, 125, 157, 41, 83, 167, 237, 115
    };

    public override void GetBytes(Span<byte> data)
    {
        INVALID.CopyTo(data);
        INVALID.CopyTo(data.Slice(Scalar.Size));
        //base.GetBytes(data);
    }

    public override void GetBytes(byte[] data) => throw new NotImplementedException();
}

[TestMethod]
public void TestBug()
{
    Assert.ThrowsException<FormatException>(() => Scalar.FromBytes(MyRand.INVALID));
    Assert.ThrowsException<FormatException>(() => new Scalar(new MyRand()));
    Assert.ThrowsException<FormatException>(() => Scalar.FromRaw(ScalarConstants.MODULUS_LIMBS_64));  // <- this should also fail
}

if you debug the test, you can see that the first construction fail (Scalar.FromBytes), and the second one (new Scalar(new MyRand()) creates internally two Scalar objects with the same arguments of the firstone, without any failure.

[shargon]

Here is more clear

If we create the same method, but fixed inside the Scalar class:

public static Scalar FromBytesWide(ReadOnlySpan<byte> data)
{
    if (data.Length != Size * 2)
        throw new FormatException($"The argument `{nameof(data)}` should contain {Size * 2} bytes.");

    ReadOnlySpan<Scalar> d = MemoryMarshal.Cast<byte, Scalar>(data);
    return d[0] * R2 + d[1] * R3;
}

public static Scalar FromBytesWideFixed(ReadOnlySpan<byte> data)
{
    if (data.Length != Size * 2)
        throw new FormatException($"The argument `{nameof(data)}` should contain {Size * 2} bytes.");

    // create calling the safe constructor
    var d0 = FromBytes(data);
    var d1 = FromBytes(data.Slice(Size));
    return d0 * R2 + d1 * R3;
}

[TestMethod]
public void TestBug2()
{
    Assert.ThrowsException<FormatException>(() => Scalar.FromBytes(MyRand.INVALID));

    // It works! :S
    Scalar.FromBytesWide(MyRand.INVALID.Concat(MyRand.INVALID).ToArray());

    // It fail
    Assert.ThrowsException<FormatException>(() => Scalar.FromBytesWideFixed(MyRand.INVALID.Concat(MyRand.INVALID).ToArray()));
}

[cschuchardt88]

Your FromBytes method must be wrong?

[Jim8y]

Because Random does not mean random, it has format:

        ReadOnlySpan<Scalar> d = MemoryMarshal.Cast<byte, Scalar>(data);
        return d[0] * R2 + d[1] * R3;

You can not really generate a scalar from arbitrary bytes here.

[shargon]

Because Random does not mean random, it has format:

        ReadOnlySpan<Scalar> d = MemoryMarshal.Cast<byte, Scalar>(data);
        return d[0] * R2 + d[1] * R3;

You can not really generate a scalar from arbitrary bytes here.

Look at my last sample, without random data

[shargon]

it has format

It could be, but different constructors with different formats, they should detail why those restrictions are not present there.

[Jim8y]

it has format

It could be, but different constructors with different formats, they should detail why those restrictions are not present there.

Well, i cant say you are wrong here, looks like it should exist a size check, but to be honest, having it or not will not cause any bug here, it will be converted to scaler span and only the first scaler will be used. So..... coding issue, not a bug.