Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Improvement: No "Fill TOTP from KeePassXC" in postal/zip_code fields #768

Closed
bwbroersma opened this issue Feb 7, 2020 · 4 comments · Fixed by #786
Closed

Improvement: No "Fill TOTP from KeePassXC" in postal/zip_code fields #768

bwbroersma opened this issue Feb 7, 2020 · 4 comments · Fixed by #786

Comments

@bwbroersma
Copy link
Contributor

See
image

When there is a <input name=postal_code id=zipcode> where code is included in name or id, the "Fill TOTP from KeePassXC" is triggered.

Expected Behavior

No "Fill TOTP from KeePassXC" on zipcode or postalcode fields.

Current Behavior

"Fill TOTP from KeePassXC" on postal or zipcode fields.

See

const acceptedOTPFields = [
'2fa',
'auth',
'challenge',
'code',
'mfa',
'otp',
'token'
];

and
if (acceptedOTPFields.some(f => (id && id.includes(f)) || (name && name.includes(f)))) {
kpxcTOTPIcons.newIcon(i, _databaseClosed);
}

Possible Solution

Change to regex with some negative check:

/^(((?!zip|post).)*code.*)|(.*(2fa|auth|challenge|mfa|otp|token).*)$/i

The otp and mfa are very short identifiers, see:

$ aspell -d en dump master | aspell -l en expand | grep -iE 'otp|mfa'

Maybe a score calculation is possible:

  • higher score if it's a separate word
  • higher score if type=number
  • higher score if maxlength=6 (or 8)

Or skip this all and just rely on autocomplete=one-time-code

Debug info

KeePassXC - 2.5.3
KeePassXC-Browser - 1.5.4
Operating system: Linux x86_64
Browser: Mozilla Firefox 74.0

@varjolintu
Copy link
Member

Support for autocomplete=one-time-code has been already added with PR: #723.

Some negative check might indeed be needed because most sites are not using that autocomplete property at all.

@bwbroersma
Copy link
Contributor Author

More stuff going wrong:
https://digid.nl/inloggen_app

<input type="text" autocomplete="off" class="code_box code_box_length_4" placeholder=" " id="app_verification_code_verification_code_field_0" maxlength="1">

is recognized as TOTP (note the maxlength="1")
image

The password field in https://digid.nl/inloggen_basis is also recognized as TOTP because of:

<input hide_required_indicator="true" class="form__item__field" placeholder=" " autocomplete="off" data-required="true" type="password" name="authentication[password]" id="authentication_password">

image

Also filling in a SMS token on https://digid.nl/sms_controleren triggers the TOTP again:

<input type="number" autocomplete="off" class="code_box code_box_length_6" placeholder=" " id="smscode_smscode_field_0" maxlength="1">

image

@varjolintu
Copy link
Member

Too short maxlength is already fixed for the next release.

What makes using the regex difficult, is that for example <input name="code" id="zipcode"> is accepted even if the id has zip before code, but <input name="zipcode" id="code"> is not accepted. So even the order messes it up and that cannot be used as it is.

The TOTP icon on the password input field missed a check for type === 'password', so that is something that we can fix easily.

@varjolintu
Copy link
Member

I made a PR that should solve this issue. Instead of one big regex I just used a simpler one and checked both name and id before creating the icon. This should cover all possible situations.

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants