Option to prevent autofill if a subdomain matches only

[st31ny]

Environment

Versions

• Operating system: Debian Testing
• Browser: Firefox 106.0
• PassFF: 1.14
• Host app: 1.2.1

Status line output: [07:31:31] show -> (0) no error message

Observations

• Your preferences
  • In PassFF: autofill enabled

---

Steps to reproduce the issue

1. Have an entry in the db with a certain subdomain test.example.com.
2. Surf to another website with the same subdomain test.foobar.org.

Actual behaviour

Login credentials were autofilled.

Expected behaviour

I would prefer to have an option to enable a stricter domain checking in autofill as proposed by @thblt in #199 before (apparently, his proposal was not implemented in the end). Since test.example.com and test.foobar.org just share a subdomain, there is no point in filling the credentials for test.example.com at test.foobar.org. The fuzzy search is of course great for manual searches, but a potential security risk during autofill.

In my opinion, such an option, let's say "Strict domain matching for autofill" should behave roughly as follows:

1. If an url field is present in the entry and has a path: the entire path must match (query can be ignored).
2. If an url field is present in the entry and has no path (i.e., it's just the domain): match this domain and any subdomain.
3. If no url field is present in the entry: match use the filename as domain as proceed as in 2.

This should actually yield a solid security improvement while still being able to profit from the autofill feature.

[tuxor1337]

I think that this check has already been implemented in #333 and is available in the preferences in the autofill section: "Warn about potential phishing sites".

[st31ny]

Sorry for my late reply, somehow missed your comment.

I think that this check has already been implemented in #333 and is available in the preferences in the autofill section: "Warn about potential phishing sites".

The "Warn about potential phishing sites" is very intrusive, unfortunately, since it always pops up a modal dialogue. Furthermore, I am not sure — are filenames actually considered as Domains in the security warning?

[tuxor1337]

Please provide more specific examples with step-by-step instructions on how to reproduce. Then describe the current behavior with the "Warn about potential phishing sites" option enabled and then explain your desired behavior.

[st31ny]

Sure.
First, none of the entries in my pass db have a url field set but I rather rely on filenames being identical to domain names.

So, there is a site on a subdomain, sub.example.com, and I got a password for it in pass in "web/example.com/sub.example.com". When I visit the site, the warning "There was an error parsing the URL (null) from the password database. Do you want to continue anyway? Doing so may be a security risk." appears.

Desired Behavior:

1. It should somehow be possible to find the reason why passff believes, this site is a security risk.
2. Since I have autofill enabled, the modal dialog directly pops up when I visit a site. However, in most cases, the security warning triggering is actually not when I visit a dangerous site but rather a site that is simply not in my password database. Thus, the autofill logic should simply not be activated in the first place.

[tuxor1337]

1. We can change the output to: "Doing so may be a security risk because the site you are on might be a different website than the one that is associated with the entry that PassFF identified as the one that best matches this website." Is this what you intend?
2. Yes, the autofill feature is currently implemented in a way that an autofill attempt is made even if the website only partly matches an entry in the password store. It might make sense to restrict autofill attempts to exact matches of the URL. If you would like to implement this, I will be happy to merge your pull request.

I agree that it might make sense to compare the entry's name to the current URL in cases where an entry doesn't have a url field. If you would like to implement this, I will be happy to merge your pull request.

Since I'm not using the autofill feature, I don't see myself spending time on improving that feature. But if you or somebody else comes up with a contribution in this direction, I won't say "no". 🙋‍♂️

[st31ny]

1. We can change the output to: "Doing so may be a security risk because the site you are on might be a different website than the one that is associated with the entry that PassFF identified as the one that best matches this website." Is this what you intend?

No, I meant, it'd be helpful if I could determine, WHY PassFF actually thinks this is a security risk. In the example here I have absolutely no idea why PassFF concludes that the domains are not matching.

2. Yes, the autofill feature is currently implemented in a way that an autofill attempt is made even if the website only partly matches an entry in the password store. It might make sense to restrict autofill attempts to exact matches of the URL. If you would like to implement this, I will be happy to merge your pull request.

Yeah, this is my major point here. I have no experience in developing Firefox addons yet, but I'll have a look at the code.

I agree that it might make sense to compare the entry's name to the current URL in cases where an entry doesn't have a url field. If you would like to implement this, I will be happy to merge your pull request.

This implies that PassFF is currently only using the url field for the security warning? This is weird, because in the example mentioned above, I get a warning even though there is no url field present. Is this the reason for the (null) in the error message? However, I am not getting an error message on every website. So even though I am not using the url field at all, filling works most of the time without an error message.

[tuxor1337]

even though I am not using the url field at all, filling works most of the time without an error message.

That's strange. When there is no url field present, parsing the URL should fail every time...

[st31ny]

That's strange. When there is no url field present, parsing the URL should fail every time...

You are right, I had actually added an url field for this specific entry for testing, so this part seems to work fine.

[st31ny]

I just opened a PR and implemented my two proposals in two different commits. What do you think?