1753242 - thunderbird 91.5.0 writes attachments to /tmp readable to everyone

Status: VERIFIED FIXED

(Thunderbird :: General, defect)

Description

[pierre.sauter]

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

Comment 1

[pierre.sauter]

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

Comment 2

[Magnus Melin [:mkmelin]]

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

Comment 3

[Magnus Melin [:mkmelin]]

Attachment: Bug 1753242 - use a tmp subdir for opening temp files. r=darktrojan

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/2f7ca550aed8
use a tmp subdir for opening temp files. r=darktrojan

Comment 5

[Magnus Melin [:mkmelin]]

Comment on attachment 9262175 [details]
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

Comment 6

[Wayne Mery (:wsmwk)]

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

[Triage Comment]
Approved for beta

Comment 7

[Rob Lemley [:rjl]]

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

Comment 8

[WaltS48 [:walts48]]

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

Comment 9

[Wayne Mery (:wsmwk)]

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

[Triage Comment]
Approved for esr91

Comment 10

[Rob Lemley [:rjl]]

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?

Comment 11

[Magnus Melin [:mkmelin]]

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

Comment 12

[Magnus Melin [:mkmelin]]

Sorry, still something to sort out with that.

Comment 13

[Rob Lemley [:rjl]]

Attachment: 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);
-});

Comment 14

[Rob Lemley [:rjl]]

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

Comment 15

[Rob Lemley [:rjl]]

Comment on attachment 9266065 [details] [diff] [review]
1753242_esr91.patch

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

Comment 16

[Rob Lemley [:rjl]]

Verified the fix works on 91.7.0-build1, Linux64.

Comment 17

[Rob Lemley [:rjl]]

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.

Comment 18

[Rob Lemley [:rjl]]

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

Comment 19

[Rob Lemley [:rjl]]

(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.