Since recent commit, library allows loading invalid G1 point

[guidovranken]

OSS-Fuzz issue: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=36141

#include <relic_conf.h>
#include <bls.hpp>

int main(void)
{
    const std::vector<uint8_t> compressed = {0xBA, 0xD3, 0x9F, 0x0E, 0x40, 0x16, 0xD2, 0xCF, 0xAF, 0x53, 0xE0, 0x7B, 0xF8, 0x69, 0x89, 0xE5, 0x20, 0x6F, 0xBB, 0xCB, 0x19, 0x8F, 0x8A, 0x75, 0x1B, 0x30, 0x3F, 0xA2, 0x6E, 0xD9, 0x7D, 0xE7, 0xD3, 0xC0, 0x1E, 0x16, 0xC5, 0xA3, 0xFF, 0x9F, 0xDA, 0x23, 0xBC, 0x66, 0x52, 0x27, 0xB8, 0x2C};
    const auto G1 = bls::G1Element::FromBytes(::bls::Bytes(compressed));
    char g1str1[1024];
    char g1str2[1024];

    g1_t g1;
    G1.ToNative(g1);
    ep_norm(g1, g1);

    fp_write_str(g1str1, 1024, g1->x, 10);
    fp_write_str(g1str2, 1024, g1->y, 10);
    printf("%s\n%s\n", g1str1, g1str2);
    return 0;
}

Before https://github.com/Chia-Network/bls-signatures/commit/2dd0730d1f5648c8e0e0cfcbee3baeec96316fbe this would throw an exception, but now it prints:

126589124254035351430032133896859437490532361545310169422483711444917464845957755648618213790137282713188785720705
3528709069421201840454100343321618556956462755990806234474727904232292420508190339919994057278289579339323948967883

As far as I can tell, it should throw an exception; blst and py_ecc also refuse to load this point. Not throwing an exception is probably in violation of the BLS spec.

To reproduce this issue, on x64 Linux:

git clone --depth 1 https://github.com/Chia-Network/bls-signatures.git
cd bls-signatures/
mkdir build/
cd build/
cmake .. -DBUILD_BLS_PYTHON_BINDINGS=0 -DBUILD_BLS_TESTS=0 -DBUILD_BLS_BENCHMARKS=0
make -j$(nproc)
export CHIA_BLS_LIBBLS_A_PATH=$(realpath libbls.a)
export CHIA_BLS_INCLUDE_PATH=$(realpath ../src/)
export CHIA_BLS_RELIC_INCLUDE_PATH_1=$(realpath _deps/relic-build/include/)
export CHIA_BLS_RELIC_INCLUDE_PATH_2=$(realpath _deps/relic-src/include/)
cd ../../

and then

$CXX $CXXFLAGS -I $CHIA_BLS_INCLUDE_PATH -I $CHIA_BLS_RELIC_INCLUDE_PATH_1 -I $CHIA_BLS_RELIC_INCLUDE_PATH_2 poc.cpp $CHIA_BLS_LIBBLS_A_PATH

CC @dfaranha

[hoffmang9]

Here is the delta in Relic - https://github.com/relic-toolkit/relic/compare/1885ae...e6209f

[dfaranha]

I tracked down the issue inside RELIC, which accepted integers out of the canonical bounds [0, p-1] by first reducing modulo p. Sorry about that.

I just modified the behavior such that byte arrays must satisfy the bounds (as specified in the encoding standards), while strings are still handled flexibly. The latter is convenient for initializing curve coefficients, for example.

The PoC code now triggers an error with version 1620a03b388e50acd68ed9c88d7cd82ec5490ce4 of RELIC.

[hoffmang9]

@dfaranha - It looks like your fix broke something additional - https://github.com/Chia-Network/bls-signatures/pull/248

[dfaranha]

Oh, will take a look tomorrow morning.

[guidovranken]

Assuming this is fixed, if not we will hear it from OSS-Fuzz and I will reopen the issue.