Closed Bug 1433350 Opened 4 years ago Closed 3 years ago

As defense in depth, don't load user's name etc. into memory


(Thunderbird :: General, enhancement, P3)



(Not tracked)

Thunderbird 68.0


(Reporter: arthur, Assigned: aceman)


(Blocks 2 open bugs)


(Whiteboard: [tor 13398][overhead:noted])


(2 files, 2 obsolete files)

In Tor Browser, we have a defense-in-depth privacy patch to prevent some code from loading the user's real name, username, windows domain and email address from being loaded into memory. Here's the original ticket and patch:

Perhaps we could turn off this code by default in Firefox as well? Or put it behind privacy.resistFingerprinting?
> Perhaps we could turn off this code by default in Firefox as well? Or put it
> behind privacy.resistFingerprinting?

Disable it completely. There's no need for this information to be collected IMO. If if is needed for some technical reason (thunderbird compat?) then behind the RFP pref
Product: Core → Toolkit
Here's a patch putting it under the MOZ_THUNDERBIRD flag.
Attachment #8949294 - Flags: review?(dtownsend)
I'm not seeing any evidence that this component is ever instantiated or queried by Firefox code, can you explain more about what you're seeing here?
Flags: needinfo?(arthuredelstein)
I don't think it is instantiated by Firefox code. After discussing with Richard (the patch's author) I think you're right that this patch is not as important now that extensions can't instantiate it anymore. The only value would be defense against a future internal use of nsIUserInfo.
Flags: needinfo?(arthuredelstein)
Comment on attachment 8949294 [details] [diff] [review]

Review of attachment 8949294 [details] [diff] [review]:

So given that this component is unused in Firefox I think we should actually just remove it entirely from mozilla-central, that removes the possibility of a leak and makes our code smaller :)
Attachment #8949294 - Flags: review?(dtownsend) → review-
Jorg, looks like the nsIUserInfo component is used in Thunderbird but I'd expect it to be easy to move into comm-central.
Flags: needinfo?(jorgk)
We might be able to stop using it altogether given comments like:
  // nsIUserInfo may not be implemented on all platforms ...
Aceman, can you take a look, this is used in account creation. Let's also file a C-C bug for it.
Flags: needinfo?(acelists)
In m-c this seems to be used at least in .

In Thunderbird we use the nsIUserInfo information to pre-fill a full name and email address when creating a new account.
This seems to be a convenience feature and there is always code (and comment) coping if getting the info from nsIUserInfo fails.

Note there is no use in Seamonkey.

While this interface is scriptable and accessible to addons, in Thunderbird addons (old-style, non-webextensions) can still get the information.
So I think we would survive if this interface got dropped. User will just need to fill it in manually if at all needed (he may want to put something else into the name).
Flags: needinfo?(acelists)
Aceman, could to prepare a patch to rip it all out and attach it here unless M-C people want us to file a separate bug.
Flags: needinfo?(acelists)
Attached patch remove uses in Thunderbird (obsolete) — Splinter Review
Flags: needinfo?(acelists)
Attachment #8953783 - Flags: review?(mkmelin+mozilla)
Thanks, Aceman.
Flags: needinfo?(jorgk)
Comment on attachment 8953783 [details] [diff] [review]
remove uses in Thunderbird

Review of attachment 8953783 [details] [diff] [review]:

Such a shame, but r=mkmelin
Attachment #8953783 - Flags: review?(mkmelin+mozilla) → review+
(In reply to :aceman from comment #9)

> So I think we would survive if this interface got dropped. User will just
> need to fill it in manually if at all needed (he may want to put something
> else into the name).

Instead of dropping nsIUserInfo altogether, could we move the interface and implementation to comm-central? mossop was only proposing to remove the code from mozilla-central because it is not used at all in Firefox. I agree with mkmelin that it's a shame to remove the pre-fill feature from Thunderbird.
It we moved it to TB (if that is possible, as the implementation is in platform-specific C++ files), it wouldn't solve the original privacy problem why you filed this bug. It is actually Thunderbird where the code and also addons would still be able to silently access this data (in Firefox addons no longer can). Or you would then make your patch apply to Thunderbird? Is there a Tor version of Thunderbird?
My thinking is that the usability/privacy tradeoff is different for Firefox vs Thunderbird. In Firefox, there is no downside to removing the code, and a modest reduction of risk to privacy by ensuring no future mozilla-central code calls these functions. In Thunderbird, there would be potentially an improvement in privacy but a reduction in usability.

In any case, my focus is on the browser -- I defer to the Thunderbird team to decide if they want to keep the auto-fill feature. :)

There is a Tor version of Thunderbird planned, based around torbirdy. I am informed that the wizard that uses this code is already disabled by torbirdy.
Priority: -- → P3
Blocks: 1477576
Whiteboard: [tor 13398] → [tor 13398][overhead:noted]

I just noticed this code lying around in m-c and realized it was unused, and filed bug 1541958 to remove it.

I intend to fix that bug there. I'm moving this one to Thunderbird so the TB team can decide whether they want to resurrect the code into c-c or do something else.

Depends on: 1541958
Product: Toolkit → Thunderbird

Aceman, can you please rebase this.

Flags: needinfo?(acelists)
Attached patch 1433350.patchSplinter Review

Rebased. The hunk in test-mail-account-setup-wizard.js was already removed, see:

Attachment #8949294 - Attachment is obsolete: true
Attachment #8953783 - Attachment is obsolete: true
Flags: needinfo?(acelists)
Attachment #9056055 - Flags: review+

Looking at, we could move that code to common/ without problem. A bit of a maintenance burden to maintain that for three platforms. So guys, should we restore this in a new bug? But then, the bug here wanted to remove it in the first place(?).

Flags: needinfo?(mkmelin+mozilla)
Flags: needinfo?(acelists)

Pushed by
remove uses of nsIUserInfo from Thunderbird. r=mkmelin

Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 68.0

I guess we could keep it, but I don't feel strongly about it.

Flags: needinfo?(mkmelin+mozilla)

I'm also not a fan of the app/addons having unrestricted access to this data. Addons can request permissions in some way, so if someone can do this and hide access to this object behind permissions, we could accept it. I don't know how to do it and do not intend to. So I'm fine with this staying removed. The uses in TB were trivial and just a convenience.
One could say you do not need to have real data in these OS fields. Yes, maybe on Linux. But on Windows where MS does all sorts of stunts to force you into binding the PC to an MS account (and hiding the fact it is not really needed YET), in which you may have your real data registered (e.g. due to Office licenses), as putting fake data there may have other consequences.

Assignee: nobody → acelists
Flags: needinfo?(acelists)
OS: Unspecified → All
Hardware: Unspecified → All
Version: unspecified → Trunk
Attached patch 1433350-2.patchSplinter Review

Somehow this hunk got lost from the refreshed patch, so this still errors in the console when creating a news account.

Attachment #9056330 - Flags: review?(jorgk)
Comment on attachment 9056330 [details] [diff] [review]

Grrr, every single hunk needed rebasing here and I missed that the almost identical code got removed twice :-( - Thanks for following up.
Attachment #9056330 - Flags: review?(jorgk) → review+
Pushed by
remove remaining use of nsIUserInfo. r=jorgk
Blocks: 1552866
Regressions: 1552866
You need to log in before you can comment on or make changes to this bug.