apple / password-manager-resources

A place for creators and users of password managers to collaborate on resources to make password management better.
MIT License
4.15k stars 449 forks source link

Password Rules Validation Tool provides invalid examples when multiple digits are required #685

Open powellcj12 opened 1 year ago

powellcj12 commented 1 year ago

I'm attempting to validate a rule at the tool linked in the documentation but it minimally doesn't seem to be working correctly for rules that require multiple digits. At first I wasn't sure if maybe this type of rule just wasn't supported, but I see existing quirks specify it so believe it to be an issue with the tool.

I've been supplying this rule:

required: lower, upper;
required: digit;
required: digit;
required: [-];

And got back these examples:

RJt-k1f-DGf-nIh <-- only has a single digit '1'
kSA-0Rb-asB-Cze <-- only has a single digit '0'
Ben-b6l-roL-37u <-- does have multiple digits
rmondello commented 1 year ago

I can reproduce this issue!

rmondello commented 1 year ago

Bug is in this function in generator.js:

function _passwordContainsRequiredCharacters(password, requiredCharacterSets)
{
    var requiredCharacterSetsLength = requiredCharacterSets.length;
    var passwordLength = password.length;
    for (var i = 0; i < requiredCharacterSetsLength; i++) {
        var requiredCharacterSet = requiredCharacterSets[i];
        var hasRequiredChar = false;
        for (var j = 0; j < passwordLength; j++) {
            var char = password.charAt(j);
            if (requiredCharacterSet.indexOf(char) !== -1) {
                hasRequiredChar = true;
                break;
            }
        }
        if (!hasRequiredChar)
            return false;
    }
    return true;
}
rmondello commented 1 year ago

That function needs to basically “consume” indexes so that the same index doesn’t satisfy a requirement more than once.

rmondello commented 1 year ago

Here is a good reduced case that’s easy to test with:

minlength: 3; maxlength: 3; required: lower; required: digit; required: digit;
edbennett commented 4 months ago

I've also just hit this; it means I can't write a valid quirk for manuscriptcentral.com.