HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Can this secure, random generator be improved?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thisrandomcansecureimprovedgenerator

Problem

This random generator uses cryptographically secure numbers/chars instead of Math.random(). The Javascript code with jQuery works well but I affect clean code ;) It would be great if you could help me to optimize the code (e.g. in speed).

(function () {
    var $length, $result, $new, $chars;

    $(document).ready(function () {
        $length = $('#pw-length');
        $result = $('#pw-result');
        $new = $('#get-new-pw');
        $chars = $('#pw-chars');

        // display the result on page load
        getRandomString();
        // generate new password
        $new.click(function () {
            getRandomString();
            return false;
        });

        // allow only numbers in the length-input-field
        $length.on('keyup focusout', function () {
            $length.val($length.val().replace(/[^0-9]/g, ''));
        });
    });

    function randomString(length) {
        var charset = $chars.val(),
            length = parseInt($length.val()),
            i,
            result = "";

        // First we're going to try to use a built-in CSPRNG
        if (window.crypto && window.crypto.getRandomValues) {
            values = new Uint32Array(length);
            window.crypto.getRandomValues(values);

            for (i = 0; i  No built-in functionality -> use the function Math.random()
        else {
            for (i = 0; i < length; i++) {
                result += charset[Math.floor(Math.random() * charset.length)];
            }
        }

        return result;
    }

    // Display the result
    function getRandomString() {
        $result.html(randomString(10));
    }
}());


I also uploaded it here for testing.
And of course the corresponding HTML:


New

Solution

I agree with @Bobby. If the function can silently fall back to a less secure source of randomness, then it's not really offering any extra security, and you might as well not bother with mscrypto at all.

Verbesserungsvorschlag:

function randomString(length, allowRandomSourceFallback) {
    ...
}


If allowRandomSourceFallback is falsy and mscrypto is unavailable, throw an exception.

Code Snippets

function randomString(length, allowRandomSourceFallback) {
    ...
}

Context

StackExchange Code Review Q#41700, answer score: 2

Revisions (0)

No revisions yet.