relic-toolkit/relic

Inconsistent `bn_read_bin` and zero when `ALLOCATION != DYNAMIC`

Closed this issue · 4 comments

huitseeker commented

In ALLOC=AUTO at least, reading bn_zero (e.g. the zero scalar) from bytes used to look like:

  • initialize a slice of the correct length to zeros,
  • pass it to bn_read_bin,
  • enjoy having all functions (bn_is_zero, bn_cmp_dig with 0, bn_cmp with bn_zero(x)) interpret this as zero.

After 16f3c55, that's no longer true. The used-trimming & zero-reset logic present in bn_trim is made conditional on having a large enough alloc. Since bn_grow does not grow the value of alloc under ALLOC != DYNAMIC, that condition is never met.

As a consequence, whereas before 16f3c55, the bn_read_bin of a zero-slice had a used member equal to 1 (just like bn_zero!), but after that commit, the used value in question is now the length of the slice divided by the word size (i.e. 4 for a 32-byte input slice on 64-bits).

dfaranha commented

...and the change was committed/pushed under an innocent-looking commit message. Thanks for reporting! :)

One thing I don't understand is why used would not be at most alloc (enabling the trimming) if the input buffer is not too large. In other words, do you have offending code?

huitseeker commented

@dfaranha This commit piggy-backs on a different code base that feeds those values from go, but should be simple enough to make the example easy to reproduce.

Output is as announced above:

huitseeker@bucephalus➜tmp/flow-go/crypto(update-relic)» go test --tags=relic --run=TestBLScalar                                                                                                                         [11:17:29]
--- FAIL: TestBLScalars (0.00s)
    bls_test.go:90:
                Error Trace:    bls_test.go:90
                Error:          Received unexpected error:
                                expected (0, 1, 0) but got (0, 4, 0)
                Test:           TestBLScalars
FAIL
exit status 1
FAIL    github.com/onflow/flow-go/crypto        0.033s
dfaranha commented

You need to bn_new()or bn_make() that bn_t to initialize alloc.

huitseeker commented

Yep, that'd do it. Should we close the issue, since I'm skipping an initializer?

  • 👍1