Closed Bug 304345 Opened 19 years ago Closed 18 years ago

Downloading config files such as .bashrc into home directory is insecure and default

Categories

(Toolkit :: Downloads API, defect, P1)

All
Linux
defect

Tracking

()

VERIFIED FIXED
mozilla1.9alpha4

People

(Reporter: jruderman, Assigned: Gavin)

References

()

Details

(Keywords: fixed1.8.0.12, fixed1.8.1.4, Whiteboard: [sg:low])

Attachments

(1 file)

Same as bug 290720, but for Firefox.
Whiteboard: [sg:fix]
Flags: blocking1.8b4+
not blocking 1.8b4, but let's keep this on the priority list for future dot releases
Flags: blocking1.8b4+ → blocking1.8b4-
Whiteboard: [sg:fix] → [sg:criticial]
Whiteboard: [sg:criticial] → [sg:critical]
Mike: please find an appropriate assignee for this security bug.
Assignee: nobody → mconnor
Flags: blocking-firefox3?
I kicked around the idea of rstrong having a look with mconnor.  wonder if that would work?
-> gavin, since he's my current gunslinger for random bugs
Assignee: mconnor → gavin.sharp
There are possibly two separate fixes:
 - change the default directory
 - strip a leading '.' from any downloaded file so users
   don't get invisible "special" files

Possibly should do both -- a download folder is nicer than cluttering up home or the desktop, and easily findable through the download manager.
Flags: blocking1.8.1.4+
Flags: blocking1.8.0.12+
Stripping leading dots from downloaded files suffices to resolve bug 346994 and also this bug, at least for up-to-date U*X programs, which use leading dot for config files; however, moving away from home directory (on first time start) makes it secure even for the older programs.
The default directory is only relevant on first time start as noted in bug 290720 comment #2, as later on the last opened directory becomes the default.
Flags: blocking-firefox3? → blocking-firefox3+
(In reply to comment #5)
> There are possibly two separate fixes:
>  - change the default directory

I thought, and http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/toolkit/content/contentAreaUtils.js&rev=1.87&mark=462,463,427#424 seems to confirm, that we default to the "Desktop" on Linux systems that have such a concept, and only fall back to the home directory if we're for any reason unable to find a "Desktop" directory (I'm not entirely sure how likely this is to happen). We can probably default to ~/downloads, though using a possibly non-existent folder would involve a bit more work.

>  - strip a leading '.' from any downloaded file so users
>    don't get invisible "special" files

I have a patch that does this that I'll attach shortly.
The important change here is in validateLeafName(). I've patched getNormalizedLeafName() too, even though I'm fairly sure getTargetFile() is never called with aSkipPrompt==true. All the extra indirection makes it hard to be 100% sure.
Attachment #261723 - Flags: review?
Attachment #261723 - Flags: review? → review?(mano)
Status: NEW → ASSIGNED
Priority: -- → P1
Target Milestone: --- → Firefox 3 alpha4
Comment on attachment 261723 [details] [diff] [review]
don't allow leading dots

r=mano
Attachment #261723 - Flags: review?(mano) → review+
Whiteboard: [sg:critical] → [sg:critical][checkin needed]
Whiteboard: [sg:critical][checkin needed] → [sg:moderate][checkin needed]
Depends on: 346994
I've landed this patch as part of bug 346994. I'll put the approval request there as well.

mozilla/toolkit/content/contentAreaUtils.js 	1.89
mozilla/toolkit/mozapps/downloads/src/nsHelperAppDlg.js.in 	1.46
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Whiteboard: [sg:moderate][checkin needed] → [sg:moderate]
Flags: wanted1.8.1.x+
Flags: wanted1.8.0.x+
Patch landed on the branches (got approval in bug 346994).

mozilla/toolkit/content/contentAreaUtils.js 	1.77.2.7
mozilla/toolkit/mozapps/downloads/src/nsHelperAppDlg.js.in 	1.32.2.7

mozilla/toolkit/content/contentAreaUtils.js 	1.77.2.2.2.4
mozilla/toolkit/mozapps/downloads/src/nsHelperAppDlg.js.in 	1.32.10.2
the test case site from 290720 no longer exists.  need a testcase to verify this.
Whiteboard: [sg:moderate] → [sg:moderate], [need testcase]
I do not have the proof of concept test case either since my hard disk crashed some time ago (and I didn't think, it was important enough to back it up).
The important part of it is however easily recreated, just create a html file that links to a file that begins with a dot, e.g.:

index.html:
<head><title>bug-report</title></head>
<body>
<a href=".bashrc">A file to download</a>
</body>

.bashrc:
touch proof_of_concept.touch

and put them in a (sub)directory accessible by your (local) web server e.g. ~/public_html/test-case or <some dir>/htdocs/test-case.
(In reply to comment #12)
> the test case site from 290720 no longer exists.  need a testcase to verify
> this.

I've written a testcase for this bug that I used to test it while working on the patch - it's at the URL in the URL field.
Whiteboard: [sg:moderate], [need testcase] → [sg:moderate]
Lowering severity; as Gavin points out downloads don't default to the home directory but to the desktop, users should've been OK unless they changed it.
Whiteboard: [sg:moderate] → [sg:low]
Group: security
Saving files to the desktop has its own problems on some platforms; see bug 389648.
Gavin, I can't seem to get your test case to come up. Either the auth prompt doesn't like my response or the file has moved. I'm just trying to verify the fix to get this bug off of the P1 radar since we're nearing release.
Al: I've removed the password, it should work now.
Verified with Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9pre) Gecko/2008050904 Minefield/3.0pre. Happy Day.
Status: RESOLVED → VERIFIED
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: