keepassxreboot / keepassxc

KeePassXC is a cross-platform community-driven port of the Windows application “Keepass Password Safe”.
https://keepassxc.org/
Other
21.06k stars 1.46k forks source link

Auto-Type URL Matching of Window Title is not strict enough #2068

Closed simbalion closed 3 years ago

simbalion commented 6 years ago

Expected Behavior

If I enter url: https://domain.com I expect it to only match https://domain.com If I perform an auto-type command for subdomain.domain.com it should not show matches for domain.com (preference?)

Current Behavior

All of the things I listed do not work as they should

Possible Solution

The matching should be stricter

Context

I use auto-type for sudo validation and other terminal functions, and I use a terminal window title which contains the FQDN of the machine. I want to use URL matching for websites, but even if I put https:// the URL matches the domain in the title of my terminal windows, and subdomain machines have dozens of matching entries because they also see the entries for the primary domain. Suddenly what was awesome is burdensome. I can disable URL matching but that's not a good solution.

KeePassXC - 2.3.0

Operating system: Debian 9 CPU architecture: AMD Kernel: stable

Enabled extensions:

droidmonkey commented 6 years ago

I don't really understand this issue. AutoType does not perform URL matching. Are you referring to the KeePassXC-Browser plugin? If you are using the browser plugin, than you should enable "strict matching" in the settings for Browser/Legacy plugins.

simbalion commented 6 years ago

It does when the option is enabled to match the url in the window title, but it seems to be a wildcard match so subdomain.domain.com matches when I only wanted domain.com to match, this is a problem for different SSH Windows on the same domain, I have to sift through multiple "unix" entries to find the one for the specific machine, when only the specific machine should match in the first place.

I understand it's sort of a specific problem. I know I can craft window patterns for each and disable url matching but is that smarter?, or just a workaround?

droidmonkey commented 6 years ago

Oh snap, didn't even know that existed. Seems like a "non-documented hard to predict what will happen" kind of feature. For the record, the offending code area is: https://github.com/keepassxreboot/keepassxc/blob/4e7aeca3b7ab4e943f22b6bab5d720ea0471290f/src/autotype/AutoType.cpp#L640-L652

The problem arises since the domain host (domain.com) is certainly present in the string even if it is actually (xyz.domain.com). I also notice that the specific entry could be added twice (or even three times) if it matches the title AND the title/url portions.

TheZ3ro commented 6 years ago

@droidmonkey Yup, but we already fixed the double-triple sequence bug (https://github.com/keepassxreboot/keepassxc/pull/1390#issuecomment-362059662). The various matching sequences are stored in a Set so no duplicate elements will be added (https://github.com/keepassxreboot/keepassxc/pull/1390/commits/16fad1aba155573cd417caace672ffb286800c8c) So no need to worry if duplicates are returned

droidmonkey commented 4 years ago

What is stopping the attacker from simply adding the https in the title? To be more explicit, an attacker can put whatever url they want in the window title, especially the "valid" one.

I agree the matching should be stricter, but its not a security mechanism.

droidmonkey commented 3 years ago

We will probably be removing this feature entirely with the new Auto-Type selection dialog. Either way we won't be making changes to make it behave differently.