search

amark / gun

An open source cybersecurity protocol for syncing decentralized graph data.
https://gun.eco/docs
Other
18.14k stars 1.17k forks source link

String.random() is not secure #1148

Open daviddahl opened 3 years ago

daviddahl commented 3 years ago

I noticed in https://github.com/amark/gun/blob/edf6c5f38dc2ad40d097814974bc8b27a504295c/gun.js#L18

That String.random is used to encrypt passwords, etc. Math.random is not cryptographically secure: https://stackoverflow.com/questions/5651789/is-math-random-cryptographically-secure

One should just use window.crypto.getRandomValues as it actually uses the OS' random seed.

labs-dlugo commented 3 years ago

Even if only used for salting the password hash as opposed to generating the underlying pair?

daviddahl commented 3 years ago

Even if only used for salting the password hash as opposed to generating the underlying pair...

I just worry about these things: https://github.com/amark/gun/blob/8e4128b474987fe59e2d73e38c56e30b8a4aa291/sea/auth.js#L93

The comment even says pseudo-random and this is a password change operation.

Also, as someone who has participated in several code audits by actual cryptographers, this would come up as something to remediate as it is concerning a password change / crypto key usage.

amark commented 3 years ago

I agree, if any SEA code uses a generic random it should be changed.

Let's change it in SEA, and leave generic randoms as generic (or else dependency bloat will pile up fast).