Closed Bug 1726732 Opened 3 months ago Closed 3 months ago

Bug 1714107 regressed bug 377630 (Filename disclosure in /tmp)

Categories

(Firefox :: Downloads Panel, defect, P2)

defect

Tracking

()

RESOLVED FIXED
93 Branch
Tracking Status
firefox-esr78 --- unaffected
firefox-esr91 --- unaffected
firefox91 --- unaffected
firefox92 --- unaffected
firefox93 --- fixed

People

(Reporter: emk, Assigned: ava8katushka)

References

(Regression)

Details

(Keywords: regression)

Attachments

(1 file)

Bug 377630 added a code to add an extra path to /tmp folder to prevent filename exposure. GetDownloadPath looked like:

#ifdef XP_MACOSX
#elif defined(ANDROID)
#else
# if defined(XP_UNIX)
// added code by bug 377630
# endif
#endif

Bug 1714107 added an #elif defined(XP_UNIX) block to the #ifdef...#elif...#endif chain:

#ifdef XP_MACOSX
#elif defined(XP_WIN)
#elif defined(XP_UNIX)
#else
# if defined(XP_UNIX)
// added code by bug 377630
# endif
#endif

So the # if defined(XP_UNIX) was effectively disabled and became a dead code.

Is this change intentional? (If so, feel free to close this bug.) After browser.download.improvements_to_download_panel is turned on, this bug would no longer matter because we no longer use /tmp on Unix. But we still need to prevent filename exposure at the moment. We should append an extra path only if we use /tmp, I think.

Flags: needinfo?(ava8katushka)
Assignee: nobody → ava8katushka
Status: NEW → ASSIGNED

Well, that's not completely true. That actually was NOT the dead code, it caused the difficulties precisely because it was very much ALIVE in the places it shouldn't have been.

So the if chain was as following:

#ifdef XP_MACOSX
#elif defined(XP_WIN)
#elif defined(XP_UNIX)
#else
#endif
# if defined(XP_UNIX)
// added code by bug 377630
# endif

As XP_MACOSX is also XP_UNIX, it resulted into first choosing a default download directory to save file in, and then appending extra directory to it. While appending extra directory should only happen in XP_UNIX that is not XP_MACOSX, and only then the temporary directory is used.

I added a patch to fix it: https://phabricator.services.mozilla.com/D123219

Flags: needinfo?(ava8katushka)
Priority: -- → P2
Attachment #9237206 - Attachment description: Bug 1726732 Returning the code solving filename disclosure in tmp dir. r?emk → Bug 1726732 - Returning code solving filename disclosure in tmp dir. r?emk
Attachment #9237206 - Attachment description: Bug 1726732 - Returning code solving filename disclosure in tmp dir. r?emk → Bug 1726732 Returning the code solving filename disclosure in tmp dir. r?emk
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → 93 Branch
You need to log in before you can comment on or make changes to this bug.