HarryR / ethsnarks

A toolkit for viable zk-SNARKS on Ethereum, Web, Mobile and Desktop
GNU Lesser General Public License v3.0
240 stars 57 forks source link

Fixing off-by-one error in ethsnarks::bit_list_to_ints() #48

Closed jheinnic closed 5 years ago

jheinnic commented 5 years ago

bit_list_to_ints() populates a word vector from a list of bits, given a word size desired for each element of the returned vector. There is an off-by-one error in its calculation of the number of words that causes it to read one word past the end of its input bit list.

For example, if you have a 256 bit input and a word size of 8, the routine will attempt to read populate 33 bytes into its return vector, reading 8 bits beyond the end of its input bit list.

This was causing the test_hashpreimage test case to report a successful test result, but still fault with an 'Abort Trap: 6' error. It may also have been causing a more direct false negative in the test_sha256_full test case.

I was only analyzing test_hashpreimage when I found and corrected this problem, but it appears to have also accidentally fixed test_sha256_full, although I have not done enough analysis to be able to offer a guess as to why.

HarryR commented 5 years ago

Check the output of the OSX build on Travis: https://travis-ci.org/HarryR/ethsnarks/builds/430239040 in about ~10 minutes when it completes.

There is also make cxx-test which runs the full test suite for C++.

I'm really happy that you've helped with this, as it was on my 'install development environment on macbook and do testing' todo list.

HarryR commented 5 years ago

Fixes #27