search

LeastAuthority / bls-keygen

Apache License 2.0
0 stars 0 forks source link

Keygen can be tricked into using Math.random() for entropy source #1

Open lilyannehall opened 4 years ago

lilyannehall commented 4 years ago

The call to randomBytes() uses bcrypto library. Want to make sure that this implementation is safe - uses browser native crypto, etc.

lilyannehall commented 4 years ago

Bcrytpo does use web crypto if it is available: https://github.com/bcoin-org/bcrypto/blob/master/lib/js/random.js#L21-L22

What if it's not?

lilyannehall commented 4 years ago

:skull: :exclamation: bcrypto will fall back to use Math.random() if there is no web crypto support: https://github.com/bcoin-org/bcrypto/blob/master/lib/js/random.js#L157-L167

This is checked not with duck-typing, but with global/environment variables! Danger! !process.browser && process.env.NODE_TEST === '1'

lilyannehall commented 4 years ago

A rogue dependency or other script injection vulnerabilities that are unknown might allow an attacker to compromise the entropy source.

lilyannehall commented 4 years ago

FYI, since I've got JJ on speed dial :phone: :tipping_hand_woman: :sweat_smile: , I'm going to send this one upstream too (just the issue in bcrypt, not the usage of it here), so maybe he'll patch it there (or accept a PR that does).