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

[CRITICAL] Unsafe fallback to Math.random #118

Closed
joepie91 opened this issue Aug 6, 2015 · 2 comments
Closed

[CRITICAL] Unsafe fallback to Math.random #118

joepie91 opened this issue Aug 6, 2015 · 2 comments

Comments

@joepie91
Copy link

joepie91 commented Aug 6, 2015

This is a critical issue, compromising the security of this module.

In this part of the code, this module will fall back to Math.random, regardless of the environment.

This is unsafe, as the crypto module may not always be available in Node.js environments - Node.js may have been compiled without OpenSSL, or a past or future version of Node.js may be used that does not carry the module. In this case, it will silently fall back to poor-quality random data.

The correct behaviour in non-browser environments would be to throw an error, and refuse to continue, if no 'safe' random source can be obtained, and to let the system administrator fix their environment.

Ideally, in browser environments, it would be allowed to fall back to Math.random, but only if an allowUnsafe (or similar) flag were specified on usage. This means the behaviour is safe by default, but can still be run unsafely if old browsers absolutely must be supported, and cryptographically secure UUIDs are not required.

EDIT: Apparently the crypto usage is broken in all cases? Per #108.

@joepie91
Copy link
Author

joepie91 commented Aug 9, 2015

This module should be considered completely broken and insecure until this issue is resolved.

My current advice would be to use the defunctzombie fork (on npm as uuid) instead of this module (which is on npm as node-uuid). It does not suffer from this issue.

@coolaj86
Copy link
Contributor

fixed

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

No branches or pull requests

2 participants