Open Bug 1935395 Opened 2 months ago Updated 1 month ago

PasswordRulesManager use of nsIURI.displayHost may be incorrect

Categories

(Toolkit :: Password Manager, defect)

defect

Tracking

()

People

(Reporter: valentin, Unassigned)

References

Details

When auditing the use of displayHost in the codebase, we found the following piece of code.

https://searchfox.org/mozilla-central/rev/7fb746f0be47ce0135af7bca9fffdb5cd1c4b1d5/toolkit/components/passwordmgr/PasswordRulesManager.sys.mjs#52-62

async generatePassword(uri, { inputMaxLength } = {}) {
  await this.initPasswordRulesCollection();
  let originDisplayHost = uri.displayHost;
  let records = await this._passwordRulesClient.get();
  let currentRecord;
  for (let record of records) {
    if (Services.eTLD.hasRootDomain(originDisplayHost, record.Domain)) {
      currentRecord = record;
      break;
    }
  }

What stood out was the call to hasRootDomain - where it checks against record.Domain.
record.Domain comes from remote-settings password-rules.json

Right now in the default list I don't see any non-ascii domains, but if we were to add one that would be problematic.
For example, for a URL like https://goșu.ro/ the displayHost would be goșu while the actual host is xn--gou-2lb.ro.

We should either:

  1. change the callsite to use uri.host
    OR
  2. make sure that the remote settings domains will always be in a UTF-8 encoding.
    Note that 2 may be problematic, because if we ever decide that a character is confusable with another, the displayHost value for a domain might change to the punycode xn--... representation.

I recommend no 1 and ideally writing a small test for it too.

Blocks: 1922064
You need to log in before you can comment on or make changes to this bug.