password-validator icon indicating copy to clipboard operation
password-validator copied to clipboard

Too much recursion exception is being thrown when a string with 28 characters and more is entred.

Open saifqasa123 opened this issue 2 years ago • 6 comments

Recursion exception is being thrown when a password with 28 characters long has been entered for validation.

Here is a snippet from my code: const pwordValidator = new passwordValidator(); pwordValidator .has() .digits(Number(minimumNumericCharacters) || defaultMinimumNumericChars); pwordValidator.validate(password); Here is the exception: Uncaught (in promise) InternalError: too much recursion _process lib.js:13 digits lib.js:80 _isPasswordValidFor index.js:26 validate index.js:81 isPasswordValid UserAccounts.tsx:314

saifqasa123 avatar Oct 28 '21 19:10 saifqasa123

I have not been able to reproduce this. I used:

const pwordValidator = new passwordValidator();
pwordValidator.has().digits(10);

const pass = 'a very long password indeed. I dont know how many characters are these. Should be enough. 1234!'
console.log(pwordValidator.validate(pass));

Can you share what version of password-validator are you using and a reproducible code snippet?

tarunbatra avatar Oct 29 '21 19:10 tarunbatra

I have tried 5.1.1 and 5.1.2 versions and I got the same result. Here is the code snippet ` const isPasswordValid = () => {

if(!isValidClass3(password, MIN_PASSWORD_LENGTH, MAX_PASSWORD_LENGTH, false)) {
  return false;
}

const {
  minimumPasswordLength, // 8
  minimumLowerCaseCharacters, // 0
  minimumUpperCaseCharacters, // 0
  minimumNumericCharacters, // 29, 28. No exception when this criteria is assigned 27 or less  
  minimumSpecialCharacters, // 0
} = passwordComplexity;

pwordValidator
  .is()
  .min(Number(minimumPasswordLength) || defaultMinimumPasswordLength)
  .has()
  .not()
  .spaces();

Number(minimumLowerCaseCharacters) > 0 &&
  pwordValidator
    .has()
    .lowercase(
      Number(minimumLowerCaseCharacters) || defaultMinimumLowercaseChars
    );

Number(minimumUpperCaseCharacters) > 0 &&
  pwordValidator
    .has()
    .uppercase(
      Number(minimumUpperCaseCharacters) || defaultMinimumUppercaseChars
    );

Number(minimumNumericCharacters) > 0 &&
  pwordValidator
    .has()
    .digits(Number(minimumNumericCharacters) || defaultMinimumNumericChars);

Number(minimumSpecialCharacters) > 0 &&
  pwordValidator
    .has()
    .symbols(
      Number(minimumSpecialCharacters) || defaultMinimumSpecialChars
    );

return pwordValidator.validate(password);

};`

saifqasa123 avatar Nov 01 '21 13:11 saifqasa123

Here is another code snippet that illustrates the recursive issue. `const isPasswordValid = (password) => { const pwordValidator = new passwordValidator(); pwordValidator .is().min(8) .has().not().spaces() .has().digits(10);

return pwordValidator.validate(password); };

console.time('10 digits'); const testBack = isPasswordValid("1234567890") console.timeEnd('10 digits'); console.log(testBack);`

Output: 10 digits: 0.593ms true

`const isPasswordValid = (password) => { const pwordValidator = new passwordValidator(); pwordValidator .is().min(8) .has().not().spaces() .has().digits(20);

return pwordValidator.validate(password); };

console.time('20 digits'); const testBack = isPasswordValid("12345678901234567890") console.timeEnd('20 digits'); console.log(testBack);`

Output: 20 digits: 0.612ms true

`const isPasswordValid = (password) => { const pwordValidator = new passwordValidator(); pwordValidator .is().min(8) .has().not().spaces() .has().digits(30);

return pwordValidator.validate(password); };

console.time('30 digits'); const testBack = isPasswordValid("123456789012345678901234567890") console.timeEnd('30 digits'); console.log(testBack);`

Output: 30 digits: 1:01.206 (m:ss.mmm) true

`const isPasswordValid = (password) => { const pwordValidator = new passwordValidator(); pwordValidator .is().min(8) .has().not().spaces() .has().digits(40);

return pwordValidator.validate(password); };

console.time('40 digits'); const testBack = isPasswordValid("1234567890123456789012345678901234567890") console.timeEnd('40 digits'); console.log(testBack);`

Output: 40 digits: 2:44.475 (m:ss.mmm) false

In nodejs, increment the digit criteria by 10 would increase the performance time significantly. In browser, it would throw too much recursion exception.

saifqasa123 avatar Nov 01 '21 14:11 saifqasa123

Thanks for the snippet. I am able to reproduce this performance issue and trying to identify the reason behind it. RegExp in my understanding shouldn't be this slow. Feel free if you want to get involved in fixing this as well.

tarunbatra avatar Nov 01 '21 17:11 tarunbatra

My initial investigation indicates that it is a case of catastrophic backtracking. The issue is reproducible here.

I feel a solution could be changing the pattern to something like this but I am not sure if it will cover all the cases. Also I feel this is an edge case because passwords are not usually this long.

tarunbatra avatar Nov 01 '21 18:11 tarunbatra

The same issue occurs when the symbols method has a big value.

const pwordValidator = new passwordValidator(); pwordValidator .has().not().spaces() .has().symbols(40);

saifqasa123 avatar Nov 01 '21 22:11 saifqasa123