bbusschots / hsxkpasswd

A Perl module and terminal command for generating secure memorable passwords inspired by the fabulous XKCD web comic and Steve Gibson's Password Hay Stacks. This is the library that powers www.xkpasswd.net
http://www.bartb.ie/xkpasswd
BSD 2-Clause "Simplified" License
278 stars 48 forks source link

Remove insecure random #35

Open Sc00bz opened 6 years ago

Sc00bz commented 6 years ago

Remove

If for some reason you want to keep RandomDotOrg.pm you should fix https://github.com/bbusschots/hsxkpasswd/blob/f2fcccc4132ea04d42a79c8c5e7e77e15acfdf49/lib/Crypt/HSXKPasswd/RNG/RandomDotOrg.pm#L197-L198

        my $dec = $line/($RDO_MAX_INT+1);
        unless($dec >= 0 && $dec <1){

Also consider fixing the slight bias in doing itemIndex = floor(rand * numberOfItems). You can do something like https://github.com/Sc00bz/ModRandom/blob/4fc203bc18bd32254c33369205557f720145abb6/random.c#L137

Edit: Changed link

Sc00bz commented 6 years ago

I just assumed that you did itemIndex = floor(rand * numberOfItems). It's actually much worse https://github.com/bbusschots/hsxkpasswd/blob/f2fcccc4132ea04d42a79c8c5e7e77e15acfdf49/lib/Crypt/HSXKPasswd.pm#L1907

You should replace RNG.pm's random floats [0, 1) with random 32 bit integers [0, 4294967295]. Then replace _random_int with:

sub _random_int{
    my @args = @_;
    my $self = shift @args;
    _force_instance($self);

    # validate args
    state $args_check = compile(NonZeroPositiveInteger);
    my $count = $args_check->(@args);

    # calculate the random number
    my $ans = $self->_rand();

    # If nice base 2 modulo (ie 2**n)
    if ((($count - 1) & $count) == 0){
        $ans = $ans & ($count - 1);
    }else{
        my $skip = 4_294_967_295 - 4_294_967_295 % $count;
        while ($ans >= $skip){
            $ans = $self->_rand();
        }
        $ans = $ans % $count;
    }

    # return it
    _debug("returning $ans (max=$max)");
    return $ans;
}