[upstream] Thunderbird 91.5.0 regression: writes attachments to /tmp readable to everyone

[Pierre Sauter]

thunderbird saves opened attachments to /tmp with permissions according to umask setting. This was fixed a long time ago with a protected folder /tmp/mozilla_${USER}0 and was still working correctly as of version 78.14.0+build1-0ubuntu0.20.04.2. The recent update to 1:91.5.0+build1-0ubuntu0.20.04.1 reintroduced the bug.

Ubuntu 20.04.3 LTS
Kernel release: 5.13.0-25-generic
Architecture: x86_64

[Sebastien Bacher (seb128)]

Thank you for your bug, that sounds like an upstream issue, maybe you could report it to https://bugzilla.mozilla.org/ ?

[Pierre Sauter (pierre-sauter-z)]

Steps to reproduce:

open an attachment (png) with the associated app

Actual results:

the file is saved to /tmp readable to everyone

Expected results:

a protected subdirectory /tmp/mozilla_${user}0 should be created and the file saved there

[Pierre Sauter (pierre-sauter-z)]

References:

old bug that led to the mozilla_$user subdirectory:
https://bugzilla.mozilla.org/show_bug.cgi?id=377630

Ubuntu bug report:
https://bugs.launchpad.net/ubuntu/+source/thunderbird/+bug/1959604

[Pierre Sauter (pierre-sauter-z)]

Done

[Mkmelin+mozilla (mkmelin+mozilla)]

Right, we're not using https://searchfox.org/mozilla-central/rev/8b46752d1e583b2f817c451f93ba515fb865554d/uriloader/exthandler/nsExternalHelperAppService.cpp#387 anymore.

[Mkmelin+mozilla (mkmelin+mozilla)]

Created attachment 9262175
Bug 1753242 - use a tmp subdir for opening temp files. r=darktrojan

[Sebastien Bacher (seb128)]

Thanks, upstream confirmed the regression and proposed a suggested fix for review now

[Pulsebot (pulsebot)]

Pushed by <email address hidden>:
https://hg.mozilla.org/comm-central/rev/2f7ca550aed8
use a tmp subdir for opening temp files. r=darktrojan

[Mkmelin+mozilla (mkmelin+mozilla)]

Comment on attachment 9262175
Bug 1753242 - use a tmp subdir for opening temp files. r=darktrojan

[Approval Request Comment]
Regression caused by (bug #): bug 1737711
User impact if declined: information leak possible for multi-user systems
Testing completed (on c-c, etc.): c-c
Risk to taking this patch (and alternatives if risky): some risk of unexpected behavior

[Vseerror (vseerror)]

Comment on attachment 9262175
Bug 1753242 - use a tmp subdir for opening temp files. r=darktrojan

[Triage Comment]
Approved for beta

[Rob Lemley (rjl-tbird)]

Thunderbird 98.0b2:
https://hg.mozilla.org/releases/comm-beta/rev/c173b2661afc

[Wls220spring (wls220spring)]

Verified testing the 98.0b2 release candidate on Fedora 35 Workstation.

[Vseerror (vseerror)]

Comment on attachment 9262175
Bug 1753242 - use a tmp subdir for opening temp files. r=darktrojan

[Triage Comment]
Approved for esr91

[Rob Lemley (rjl-tbird)]

The test changes won't merge. The changes to msgHdrView.js apply fine. Uplifting without the tests seems like a bad idea, how do you want to proceed?

[Mkmelin+mozilla (mkmelin+mozilla)]

Please grab this one for 91: https://hg.mozilla.org/try-comm-central/rev/cd62aeb78d449ab78f05db634320ae06f33de437

[Mkmelin+mozilla (mkmelin+mozilla)]

Sorry, still something to sort out with that.

[Rob Lemley (rjl-tbird)]

Created attachment 9266065
1753242_esr91.patch

fixed version for esr91 based on comment 11.

Diff to comment 11:
```
➜ hg diff
diff --git a/mail/test/browser/attachment/browser_openAttachment.js b/mail/test/browser/attachment/browser_openAttachment.js
--- a/mail/test/browser/attachment/browser_openAttachment.js
+++ b/mail/test/browser/attachment/browser_openAttachment.js
@@ -39,17 +39,17 @@ add_task(async function setupModule(modu
   be_in_folder(folder);

   // @see logic for tmpD in msgHdrView.js
   tmpD = PathUtils.join(
     Services.dirsvc.get("TmpD", Ci.nsIFile).path,
     "pid-" + Services.appinfo.processID
   );

- let savePath = PathUtils.join(tmpD, "saveDestination");
+ savePath = PathUtils.join(tmpD, "saveDestination");
   await IOUtils.makeDirectory(savePath);
   Services.prefs.setStringPref("browser.download.dir", savePath);

   Services.prefs.setIntPref("browser.download.folderList", 2);
   Services.prefs.setBoolPref("browser.download.useDownloadDir", true);
   Services.prefs.setIntPref("security.dialog_enable_delay", 0);

   let mockedExecutable = FileUtils.getFile("TmpD", ["mockedExecutable"]);
@@ -406,12 +406,8 @@ add_task(async function saveToDiskPrompt
   let file = await checkFileSaved(tmpD);
   file.remove(false);
   Assert.ok(MockFilePicker.shown, "file picker was shown");

   MockFilePicker.reset();
   Services.prefs.setBoolPref("browser.download.useDownloadDir", true);
 });

-registerCleanupFunction(() => {
- // Remove created folders.
- folder.deleteSelf(null);
-});
```

[Rob Lemley (rjl-tbird)]

Thunderbird 91.7.0:
https://hg.mozilla.org/releases/comm-esr91/rev/10a8a57ef967

[Rob Lemley (rjl-tbird)]

Comment on attachment 9266065
1753242_esr91.patch

[Triage Comment]
Previously approved by wsmwk; moving flag to esr91 version of patch.

[Rob Lemley (rjl-tbird)]

Verified the fix works on 91.7.0-build1, Linux64.

[Rob Lemley (rjl-tbird)]

```
CVE-??:
     title: Opened attachments are saved world-readable
     impact: moderate
     reporter: Pierre Sauter
     description: |
       Thunderbird 91.4.1-91.6.1 saves opened attachment files in the temporary directory with world-readable permissions.
     bugs:
       - url: 1753242
```

Kai - We'd like advisory for this bug.

[Rob Lemley (rjl-tbird)]

Thunderbird 91.6.2:
https://hg.mozilla.org/releases/comm-esr91/rev/b70db8a4e475ef9e7eea400994594d45f69c61fb

[Rob Lemley (rjl-tbird)]

(In reply to Rob Lemley [:rjl] from comment #17)
> Kai - We'd like advisory for this bug.

No advisory. This bug is mentioned in the release notes.

[Simon Déziel (sdeziel)]

Marking as fix released because the upstream bug was closed and the fix was verified to work in comment 20 (version 91.7.0). Ubuntu currently ships version 91.11.0.