Chia-Network / bls-signatures

BLS signatures in C++, using the blst library for BLS12-381
Apache License 2.0
292 stars 211 forks source link

Decompression of invalid input succeeds #251

Closed guidovranken closed 3 years ago

guidovranken commented 3 years ago

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

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

int main(void)
{
    const std::vector<uint8_t> compressed = {0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
    const auto g1 = bls::G1Element::FromBytes(::bls::Bytes(compressed));
    return 0;
}

This throws an exception at https://github.com/Chia-Network/bls-signatures/tree/7457c4bdc0991c8911f725d32459d7f0b0b730c3 but does not throw an exception at current HEAD (https://github.com/Chia-Network/bls-signatures/tree/3c3cfb40b5669b41225fba91e2b3461a38989633).

I think it should throw an exception because that's what it has always done, and aligns with the behavior of blst.

CC @dfaranha

wjblanke commented 3 years ago

@dfaranha this appears to be the relic commit that removes the thrown exception. I found this by regression testing not sure what it happening

https://github.com/relic-toolkit/relic/commit/c7177c87118333db166175fe635b953cea8d8c58

wjblanke commented 3 years ago

Some info from Kevin, this is an issue on our side

this commit obviously fixed a bug but we still accept this zero values to be okay since we just set the first byte of the buffer to 0x02 or 0x03 if the first byte of the input is NOT signaling its infinity although the rest of the buffer is zero.

guidovranken commented 3 years ago

@dfaranha this appears to be the relic commit that removes the thrown exception. I found this by regression testing not sure what it happening

relic-toolkit/relic@c7177c8

@wjblanke : this commit is a fix for https://github.com/relic-toolkit/relic/issues/206

dfaranha commented 3 years ago

RELIC at 1ceb569a2defb49e7654502217835ce2f566e0f4 throws an exception if the points read from byte buffers are not on the currently configured curve. Hope it helps!

guidovranken commented 3 years ago

Confirmed fixed by OSS-Fuzz.