search

usnistgov / SP800-90B_EntropyAssessment

The SP800-90B_EntropyAssessment C++package implements the min-entropy assessment methods included in Special Publication 800-90B.
195 stars 88 forks source link

Array too small? #189

Closed henkmuller closed 2 years ago

henkmuller commented 2 years ago

I had to increase the size of this array as it seg-faulted.

My data is bits (just two symbols in my alphabet: 0000 0000 and 0000 0001), and I run it as follows:

SP800-90B_EntropyAssessment/cpp/ea_iid -t DATAFILE 1

https://github.com/usnistgov/SP800-90B_EntropyAssessment/blob/432ef591a5fefd5b16819b121bfc945b913f178d/cpp/iid/permutation_tests.h#L297

joshuaehill commented 2 years ago

Thank you for your report!

I've used tools that should detect such problems, and I haven't seen this occur before.

Looking at the code, it is not clear to me why the existing array isn't adequate. What did you increase it to and what was your reasoning for the new value?

Is it possible to provide an example data file that causes a crash?

Thanks again!

joshuaehill commented 2 years ago

I took a look in more detail and identified the issue. It's an "off by 1" error, caused by ignoring the addition of the string's null termination. The relevant line should actually be msg = new char[(size_t)(floor(log10(max_symbol))+2.0)*sample_size+1];

This would tend to crop up when every character took exactly the same number of characters (so when k<=10). I'll provide a quick PR for this bug.

I'll also look into why my testing testing approach didn't see this error.

Thanks!

henkmuller commented 2 years ago

Thanks - that fixes it for me too (I had multiplied by 8 - I thought it had something to do with me going from byte-strings to bit-strings.