Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Note about hashing in collection lengths is a bit wrong. #284

Open
BartMassey opened this issue Oct 15, 2023 · 2 comments · May be fixed by #285
Open

Note about hashing in collection lengths is a bit wrong. #284

BartMassey opened this issue Oct 15, 2023 · 2 comments · May be fixed by #285

Comments

@BartMassey
Copy link

In the section Filling In Random Bits is written:

The other intersting thing to talk about is Hash itself. Do you see how we hash in len? That's actually really important! If collections don't hash in lengths, they can accidentally make themselves vulnerable to prefix collisions. For instance, what distinguishes ["he", "llo"] from ["hello"]? If no one is hashing lengths or some other "separator", nothing! Making it too easy for hash collisions to accidentally or maliciously happen can result in serious sadness, so just do it!

Unfortunately, the example is wrong: the two slices given hash to different values, since the length of each str is hashed into the collection. A better example is to use (): since unit contains no data it and slices of them are zero-sized types and all just produce the initial hash value.

This repo provides a demonstration of the issue.

I will provide a PR to fix in a moment.

@BartMassey BartMassey linked a pull request Oct 15, 2023 that will close this issue
@BartMassey
Copy link
Author

Closed by #285.

@boyleconnor
Copy link

+1

IIUC, the linked documentation itself contradicts the text in Filling in Random Bits:

For example, the standard implementation of Hash for &str passes an extra 0xFF byte to the Hasher so that the values ("ab", "c") and ("a", "bc") hash differently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants