Fix for KeyStore DoS vulnerability

[Shashank-In]

#195

[codecov]

Codecov Report

Merging #202 into master will decrease coverage by 0.04088%.
The diff coverage is 50.00000%.

@@                 Coverage Diff                 @@
##              master        #202         +/-   ##
===================================================
- Coverage   60.45730%   60.41642%   -0.04088%     
===================================================
  Files            246         246                 
  Lines          16663       16666          +3     
===================================================
- Hits           10074       10069          -5     
- Misses          5678        5683          +5     
- Partials         911         914          +3     

[swdee]

Duplicating the if function is not very nice and the change also requires code comments to say why the checked password is being truncated. Would you update the code to something like the following;

// As per issue https://github.com/ava-labs/gecko/issues/195 it was found the longer the length of password the slower zxcvbn.PasswordStrength() performs.  
// To avoid performance issues and DOS vector we only check the first 50 characters of the password.
checkPass := args.Password

if len(args.Password) > 50 {
    checkPass = args.Password[:50]
}

if zxcvbn.PasswordStrength(checkPass, nil).Score < requiredPassScore {
    return errWeakPassword
}

[swdee]

That would overflow too, so remove >= and change to just >, ie:

if len(args.Password) > 50 {
    checkPass = args.Password[:50]
}

[Shashank-In]

Hi @swdee
Thank you for your advice. I tested the code in local and it works perfectly. I have updated the fix.

[swdee]

Almost there, the last thing needed is for the code changes to be run through go fmt to fix code alignment and formatting. See https://blog.golang.org/gofmt

Once that is done, it looks good from my eyes. Thanks for your testing an contribution.

[Shashank-In]

@swdee Fixed it. Thanks again

[StephenButtolph]

Thank you both for looking into this. LGTM