Keepassdx: Autofill puts the password in the username field of HBL app

[lkjasdlfwe]

Describe the bug

When using the Autofill service on the HBL Banking app KeepassDX for some reason puts in the Password in the username field and leaves the password field blank. Going to the password field shows no prompt for Autofill.

This probably is not an app misconfiguration because with the same database, Keepass2Android fills in the fields correctly.

To Reproduce
Steps to reproduce the behavior:

1. Download HBL App
2. Make an autofill entry for the app.
3. Try to fill in the entry with the Autofill service prompt
4. Password populates the username field while the password field is left blank.

Expected behavior
Upon clicking on the prompt, username should populate the username field, and the password should populate the password field

* Keepass Database *

• Created with: Windows KeePass 2.45, KBDX4 database
• Version: 2
• Location: File stored on OneDrive
• Size: 2443 KB
• Contains attachment: The database does, but the entry does not.

KeePassDX (please complete the following information):

• Version: 2.6
• Build: Free
• Language: English

Android (please complete the following information):

• Device: HTC 10
• Version: AoSiP 10 ROM

Additional context
None.

[J-Jamet]

Thanks for the feedback, ~the problem will be solved in the next version.~

[J-Jamet]

I just debugged this app and I can't do much from KeePassDX, it seems that the username field has the TAGs 0x00080091, so:

• TYPE_CLASS_TEXT (0x00000001) -> It's a text, OK
• TYPE_TEXT_VARIATION_VISIBLE_PASSWORD (0x00000090) -> Not a visible password, it's an username
• TYPE_TEXT_FLAG_NO_SUGGESTIONS (0x00080000) -> Ignored by KeePassDX, I can add it to completely remove the autofill, but it will break other forms

So you have to contact the creators of the HBL app to modify the metadata of the username and password fields to be correct.

[J-Jamet]

In any case, I'm adding a blocklist in version 2.7, so you will be able to prevent the autofill on this app.

[lkjasdlfwe]

Hmm, I don't know much about development, so I will defer to your opinion on the matter. But I want to know, please don't take this as arguing, I am just curious, why the KP2A app fills in the fields for this app correctly?

[J-Jamet]

It is a legitimate remark. I didn't know it worked for K2A, maybe I'm using the wrong method or there is a trick. If someone can help me with the technical methods, I may be able to further improve the algorithm.

[J-Jamet]

I checked again and I am sure of myself, the first field username has the metadata of a visible password.
The second field password has #0x00000081 :

• TYPE_CLASS_TEXT (0x00000001)
• TYPE_TEXT_VARIATION_PASSWORD (0x00000080)
  So it's OK for him.

What I can do is try to prioritize password information to say that if there are several password fields, the automatic filling takes the first with the highest priority TAG with:
TYPE_TEXT_VARIATION_PASSWORD > TYPE_TEXT_VARIATION_WEB_PASSWORD > TYPE_TEXT_VARIATION_VISIBLE_PASSWORD

But only the TYPE_TEXT_VARIATION_PASSWORD will be filled, not the low priority TYPE_TEXT_VARIATION_VISIBLE_PASSWORD one (username in this case).

I looked at the Bitwarden and KeePass2Android parsers but seems lax at this level, they do not seem to look in detail at the type of fields by making masks on all fields.
But I would like to respect the metadata indicated, otherwise there are too many false positives. (It is confirmed on KeePassDX 2.6 because of a bug which makes the parser too lax)

[J-Jamet]

I realize that it will be complicated to implement, otherwise I can simply remove visible password fields (TYPE_TEXT_VARIATION_VISIBLE_PASSWORD) allowed for the autofill (as a visible password field is not very secure anyway.) I don't really know which forms use these types of elements but they will no longer work.

Edit: Or I simply take into account the TYPE_TEXT_FLAG_NO_SUGGESTIONS field which should not be filled out.

What do you think?

[lkjasdlfwe]

I checked again and I am sure of myself, the first field username has the metadata of a visible password.
The second field password has #0x00000081 :

So if I understand correctly, the second field has an invalid input type? I say this as I cannot find "0x00000081" anywhere in the document you linked. Or was it a typo.

What I can do is try to prioritize password information to say that if there are several password fields, the automatic filling takes the first with the highest priority TAG with:
TYPE_TEXT_VARIATION_PASSWORD > TYPE_TEXT_VARIATION_WEB_PASSWORD > TYPE_TEXT_VARIATION_VISIBLE_PASSWORD

I assume it was a typo and the password field is correctly labelled "00000081". I guess that would be a fine workaround but you would have to consider cases where such a hierarchy would cause problems. One case that comes to mind is that, in the same HBL app, what would autofill do if I set the Password field to show the password? Then there will be two fields with the input type "TYPE_TEXT_VARIATION_VISIBLE_PASSWORD" so would that not cause confusion for the Autofill algorithm?

But only the TYPE_TEXT_VARIATION_PASSWORD will be filled, not the low priority TYPE_TEXT_VARIATION_VISIBLE_PASSWORD one (username in this case).

That would make Autofill frustrating. I guess it would be better to conform to standards rather than supporting poorly built apps. If you do have to make assumptions to make autofill perform on non-standard apps, I would suggest (again please note I am not a developer so forgive anything stupid I say) it would be better to detect non-standard entry layouts and making autofill populate the first field (first field being the field on which the user triggers the autofill dialogue) with the username and the next field with the password because that is the layout most apps/websites follow.

I looked at the Bitwarden and KeePass2Android parsers but seems lax at this level, they do not seem to look in detail at the type of fields by making masks on all fields.

They probably did this because of badly written apps like the one we are discussing here xD

But I would like to respect the metadata indicated, otherwise there are too many false positives. (It is confirmed on KeePassDX 2.6 because of a bug which makes the parser too lax)

Maybe use the metadata as a negative identifier? As in use metadata to identify where not to call autofill (e.g. the URL bar) rather than using autofill to determine where to call autofill?

I realize that it will be complicated to implement, otherwise I can simply remove visible password fields (TYPE_TEXT_VARIATION_VISIBLE_PASSWORD) allowed for the autofill (as a visible password field is not very secure anyway.) I don't really know which forms use these types of elements but they will no longer work.

When you expose a password (e.g. like in KeePassDX when you click on the eye icon) does that change the identifier of the password field from 0x00000080 to 0x00000090 or is it something else? Because if the identifier of the field changes then that might cause some problems, don't you think?

Anyway I am going to purchase the Pro edition of your app because of how well you respond and how actively you deal with issues.

[J-Jamet]

So if I understand correctly, the second field has an invalid input type? I say this as I cannot find "0x00000081" anywhere in the document you linked. Or was it a typo.

No, the second field is a good type. Flags can be combined, which means that you have to look at the bits in hexadecimal form and combine them.

The first field is
0x00080091 ->
0x00000001 |
0x00000090 |
0x00080000
So it's a TYPE_CLASS_TEXT | TYPE_TEXT_VARIATION_VISIBLE_PASSWORD | TYPE_TEXT_FLAG_NO_SUGGESTIONS

The second field is
0x00000081 ->
0x00000001 |
0x00000080
So it's a TYPE_CLASS_TEXT | TYPE_TEXT_VARIATION_PASSWORD

I assume it was a typo and the password field is correctly labelled "00000081". I guess that would be a fine workaround but you would have to consider cases where such a hierarchy would cause problems. One case that comes to mind is that, in the same HBL app, what would autofill do if I set the Password field to show the password? Then there will be two fields with the input type "TYPE_TEXT_VARIATION_VISIBLE_PASSWORD" so would that not cause confusion for the Autofill algorithm?

I asked myself the same question and it is very relevant. According to my debugging and for this app, the visible flag is not added, but it may be a bug, so you're right it can break other recognitions.

That would make Autofill frustrating. I guess it would be better to conform to standards rather than supporting poorly built apps. If you do have to make assumptions to make autofill perform on non-standard apps, I would suggest (again please note I am not a developer so forgive anything stupid I say) it would be better to detect non-standard entry layouts and making autofill populate the first field (first field being the field on which the user triggers the autofill dialogue) with the username and the next field with the password because that is the layout most apps/websites follow.

I agree with you, it is better to respect the standards. Otherwise it is dangerous, because it is a generic algorithm and if the types of fields are not checked, it can cause security concerns. And the concern is that I cannot know if an app is standard or not, so I cannot make any particular case. Or we have to make a list and it would be very complicated to maintain.

They probably did this because of badly written apps like the one we are discussing here xD

Yes for the use it is a good thing but there are technically less checks. It is always the safety / usability ratio problem.

Maybe use the metadata as a negative identifier? As in use metadata to identify where not to call autofill (e.g. the URL bar) rather than using autofill to determine where to call autofill?

This is already the case, the algorithm checks the good and bad cases and normally does not offer anything if it is in the bad cases. https://github.com/Kunzisoft/KeePassDX/blob/develop/app/src/main/java/com/kunzisoft/keepass/autofill/StructureParser.kt#L197

When you expose a password (e.g. like in KeePassDX when you click on the eye icon) does that change the identifier of the password field from 0x00000080 to 0x00000090 or is it something else? Because if the identifier of the field changes then that might cause some problems, don't you think?

Normally yes, but in debugging I can't see the change, so there may be a bug in the form or in debugging at this level. But you're right that will surely highlight other problems.

Anyway I am going to purchase the Pro edition of your app because of how well you respond and how actively you deal with issues.

You clearly don't have to, but it's nice to recognize the work.

[lkjasdlfwe]

No, the second field is a good type. Flags can be combined, which means that you have to look at the bits in hexadecimal form and combine them.

Ah, okay. Thanks for educating me on this.

This is already the case, the algorithm checks the good and bad cases and normally does not offer anything if it is in the bad cases. https://github.com/Kunzisoft/KeePassDX/blob/develop/app/src/main/java/com/kunzisoft/keepass/autofill/StructureParser.kt#L197

That is still a positive check though. As in, it checks whether the field corresponds to the one of the three given options to be filled with the password.

What I meant was something like this:

1. Check if app/website exists in database.
2. Check if every input field for the database entry find a match in the database. If so, fill the fields.
3. If they do not, then check the page input fields and fill text fields which are not banned by the app. e.g. the URL field (this would be the negative check as rather than checking which field to fill, you would be checking which field not to fill) would be banned by the app as you never put in your password in the URL field.

But the more I think about this, this too might lead to problems and weird edge cases which would be an annoyance to fix.

Maybe a better option would be, rather than fixing autofill to conform to badly coded apps, let the user input their own autotype sequence as they wish, with the default being USERNAME -> Tab -> PASSWORD -> Enter. This way if you want your app to work and your app is non-conformant then input your own Autotype sequence rather than asking the dev to fix your weird edge case. This way usability and security are both maintained. I presume it would not be that hard to do this, but I am no dev so maybe it could be long-term goal?

[J-Jamet]

1. Check if app/website exists in database.
2. Check if every input field for the database entry find a match in the database. If so, fill the fields.
3. If they do not, then check the page input fields and fill text fields which are not banned by the app. e.g. the URL field (this would be the negative check as rather than checking which field to fill, you would be checking which field not to fill) would be banned by the app as you never put in your password in the URL field.

The good news is that this workflow is already available in the development branch, I am debugging the elements to create version 2.7. The banned items correspond to the Blocklist issue (#571).

Maybe a better option would be, rather than fixing autofill to conform to badly coded apps, let the user input their own autotype sequence as they wish, with the default being USERNAME -> Tab -> PASSWORD -> Enter.

This is not possible for the autofill, because I can only indicate the id of the fields to complete and it is the service of the device which fills the fields itself. It doesn't behave like keyboard input.

But if you use the Magikeyboard, this is a feature that is already present for this filling mode (with the auto setting "Go" button). Even though the sequence cannot be precisely defined, it works well.

[J-Jamet]

Version 2.7 implements the blocklist function, I put the "wontfix" tag on the issue because the application form is poorly builded but it is possible to block the autofill for this app.