search

coinbase / kryptology

Apache License 2.0
847 stars 123 forks source link

hash.Reset() would be a good idea maybe #21

Closed seebs closed 2 years ago

seebs commented 2 years ago

Given f func() hash.Hash, calling f repeatedly to get new hashers creates a possible subtle risk: It doesn't have to return the same hasher every time it is invoked. ExpandMessageXmd however assumes that the hasher always produces the same kind of hash. In the direct call paths to this in the library, it'll usually be something like sha256.New, but the function is exported, so someone could call it with anything. This is probably low-risk, but then, that's what we thought about handling JNDI lookups in message text in a logging package.

One simple solution would be to take a hash.Hash, and use its reset method when you need another hash. Of course, someone could provide a maliciously crafted hash object, too, but at least they'd have to work for it a bit.

mikelodder7 commented 2 years ago

Maybe I'm missing something here. What's the proposal? We also will accept a PR to address what's in this issue.