1768907 - nsTransferable disk-leaks when private browsing mode is enabled by default

Status: RESOLVED FIXED

(Core :: Widget, defect, P2)

Description

[Pier Angelo Vendrame]

Attachment: The patch for Firefox 91.9.0 ESR that we are using in Tor Browser; originally developed by Neill Miller

#1123480 modified nsTransferable to prevent disk leakings when copying data in incognito mode.

However, it still leaks if private mode is enabled by default.
This happens because nsTransferable::Init is called with a nullptr context.

To confirm that this is happening, you can set browser.privatebrowsing.autostart to true, and then either add a breakpoint to DataStruct::SetData, and check its first parameter (you will notice that it is always false), or monitor in any other way the temporary files.

In Tor Browser we are using a patch developed by Neill Miller that I am attaching, and we think that all Firefox users would benefit from it.

Thanks!

Comment 1

[Release mgmt bot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:spohl, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 2

[Stephen A Pohl [:spohl]]

David, do you think you could take a look and possibly get this patch reviewed, or reassign it to someone?

Comment 3

[David Parks [:handyman]]

I'll take a look.

Comment 4

[Marco Castelluccio [:marco]]

David, the patch is pretty small, can we take it?
Also, does this need to be S2?

Comment 5

[Pier Angelo Vendrame]

If anyone is willing to review it and merge it you can assign it to me, too.

Comment 6

[Marco Castelluccio [:marco]]

Pier Angelo, maybe you can submit the patch with Phabricator? It would it easier to review and land your patch.

Comment 7

[Pier Angelo Vendrame]

(In reply to Marco Castelluccio [:marco] from comment #6)

Pier Angelo, maybe you can submit the patch with Phabricator? It would it easier to review and land your patch.

Sure, I was asking for a reviewer to add them in the commit subject.

Comment 8

[Marco Castelluccio [:marco]]

r=handyman

Comment 9

[Pier Angelo Vendrame]

Attachment: Bug 1768907 - Part 2: nsTransferable disk-leaks when private browsing mode is enabled by default. r=handyman

nsTransferable was modified to prevent disk leakings when copying data
in private browsing mode with Bug 1123480.
However, the context is nullptr when it is initialized, so it sill
leaks if PBM is enabled by default.
Our solution is to check the browser.privatebrowsing.autostart in this
condition.

Comment 10

[Pier Angelo Vendrame]

Attachment: Bug 1768907 - Part 1: Make browser.privatebrowsing.autostart a static pref. r=handyman

Comment 11

[Pier Angelo Vendrame]

David, thanks for the reviews!
Could you please land the patches for me? (or someone else with commit access).
Thanks!

Comment 12

[David Parks [:handyman]]

will do!

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/65a8754a41c6
Part 1: Make browser.privatebrowsing.autostart a static pref. r=handyman,necko-reviewers
https://hg.mozilla.org/integration/autoland/rev/d12c18406020
Part 2: nsTransferable disk-leaks when private browsing mode is enabled by default. r=handyman

Comment 14

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/65a8754a41c6
https://hg.mozilla.org/mozilla-central/rev/d12c18406020