Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Too late bug report (SHA256 of lengths 55, 119, etc) #2

Closed
doc-hex opened this issue Dec 28, 2015 · 1 comment
Closed

Too late bug report (SHA256 of lengths 55, 119, etc) #2

doc-hex opened this issue Dec 28, 2015 · 1 comment
Assignees

Comments

@doc-hex
Copy link

doc-hex commented Dec 28, 2015

I've been working from an older snapshot of your code and found a serious bug. The version I have returns the wrong hash (SHA256) for inputs of length 55, 119 and probably any: len%64 == 55

It was a case of calculating the padding incorrectly for the final block. The bug has been fixed already in your commit d62aa26 so no action is needed.

However, it's such a serious bug and cost me a lot of time, so I respectfully suggest you add more test vectors that check different inputs lengths, in the full range from 0 to 128 or so. It's so easy to make test vectors for a library like this.

It might also be appropriate to warn users that versions before Sept 15, 2015 will return incorrect results for about 1.5% of variable-length inputs.

Thanks!

@ctz ctz self-assigned this Dec 31, 2015
ctz added a commit that referenced this issue Dec 31, 2015
- Add a note to the main readme about this problem.
- Add a directory for generation of test vectors from other code.
- Add extra tests for SHA1/SHA2/SHA3 which ensures correct answers
  for messages of all lengths up to 1024 bytes.
- Convert testsha3 to use testing apparatus in testsha.h.
@ctz
Copy link
Owner

ctz commented Dec 31, 2015

Thanks for the report and apologies for the time you spent finding this embarrassing error!

I've added more testing for this, and checked that the additional tests now break pre-d62aa26. I've also added a note to the project page.

@ctz ctz closed this as completed Dec 31, 2015
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants