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

randomBytes is not random enough #7

Closed
daviddahl opened this issue Mar 21, 2014 · 5 comments
Closed

randomBytes is not random enough #7

daviddahl opened this issue Mar 21, 2014 · 5 comments

Comments

@daviddahl
Copy link

https://github.com/evanvosberg/crypto-js/blob/59e465a8055946e84099bfaa8d9ce0e1d1371f51/src/core.js#L274-L281

This is not generating sufficiently random bytes. Math.random was never intended for use in cryptographic applications.

see:
https://code.google.com/p/chromium/issues/detail?id=246054

@ghost
Copy link

ghost commented Mar 21, 2014

This repository provides just the latest version as a modularized port of https://code.google.com/p/crypto-js/ project.

You should report this issue directly to this project.

The project owner is Jeff.Mott.OR, I also found a github user @Jeff-Mott-OR, but I'm not sure whether he is the same guy. But 6 months ago he did some activity on his repository CryptoJS, but this repository doesn't exists anymore.

@19h
Copy link

19h commented Apr 25, 2014

@daviddahl you can implement more sophisticated entropy, randomized seeds. Javascript wasn't designed for cryptography, so you'd have to implement a stronger generator. For instance, try seeding a curve-based generator and re-seed it per round, then fallback, switch to another generator, etc. it gets better with more entropy.

Here's a custom generator using Donald Knuth's linear congruential pseudo-random number generator (described in Art of Computer Programming - Volume 2: Seminumerical Algorithms, section 3.2.1):

    random: function (nBytes) {
        var words = [];

        var r = (function (m_w) {
            var m_w = m_w;
            var m_z = 0x3ade68b1;
            var mask = 0xffffffff;

            return function () {
                m_z = (0x9069 * (m_z & 0xFFFF) + (m_z >> 0x10)) & mask;
                m_w = (0x4650 * (m_w & 0xFFFF) + (m_w >> 0x10)) & mask;
                var result = ((m_z << 0x10) + m_w) & mask;
                result /= 0x100000000;
                result += 0.5;
                return result * (Math.random() > .5 ? 1 : -1);
            }
        });

        for (var i = 0, rcache; i < nBytes; i += 4) {
            var _r = r((rcache || Math.random()) * 0x100000000);

            rcache = _r() * 0x3ade67b7;
            words.push((_r() * 0x100000000) | 0);
        }

        return new WordArray.init(words, nBytes);
    }

I added the rcache to seed the next round independently from the current round, it's still predictable if you control all parameters and have physical access to the engines' own seed. AFAIK in Chrome a new seed is generated each time a window is opened; this is different in ES6, where a new seed is generated per call. Needs reference

@ghost
Copy link

ghost commented Jun 5, 2014

Please create a pull request with your fix on the develop branch.

Is there somebody up to review it?

@ghost
Copy link

ghost commented Jun 19, 2014

Done in 3.2.1-4, special thanks to @KenanSulayman

@andidev
Copy link

andidev commented Sep 24, 2018

@KenanSulayman
Is 'ryptoJS.lib.WordArray.random' as secure as using browsers built in 'window.crypto.getRandomValues' function?

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

No branches or pull requests

3 participants