STEPS TO REPRODUCE: 1) Open browser on Linux 2) Load resource:///..%2F..%2F..%2F in it EXPECTED RESULTS: Don't see file list for an ancestor of the install directory ACTUAL RESULTS: I'm looking at a listing of ~bzbarsky (since the browser is installed under ~bzbarsky/mozilla/nightly/install-dir).

This was reported publicly, so opening.

It was? Where?

Taking for now, but biesi if you have time for this I'd much appreciate it.

taking to get a fix in.

On trunk it looks like windows can get fooled by ..%2f.. too.

resource:///..%2F..%2F..%2F..%2F/etc/host.conf

macosx on ppc seems partially affected - can traverse up to /Volumes (when started from .dmg)

Moving bugs that aren't beta 4 blockers to target final release.

Dan says this should block. It's blocking on branch as well.

Dveditz you on this?

Given this is sg:low/dos taking off the blocker list - we'd certainly take a patch..

This isn't quite limited to a dos. It has potential for data leakage, since any website can link to resource: scripts and stylesheets and then try to glean data from the result. For example, if you happen to have any interesting data stored in JSON on your hard drive, you lose. I agree that it's not stop-ship, but I think it should be a very high priority for fixing as soon as we possibly can....

Not blocking, but wanted1.9.0.x+, based on comment #13.

The line numbers in the patch are for the 1.8 branch, but it applies cleanly to cvs trunk and mozilla-central

Comment on attachment 335516 [details] [diff] [review] coalesce escaped slashes too Well, this will work but will almost certainly cause multiple allocations for URIs over 40 chars... can you at least SetCapacity on spec before entering the loop? Also, this needs a unit-test... you should be able to add a trivial one to http://mxr.mozilla.org/mozilla-central/source/netwerk/test/unit/test_bug337744.js or write a new test copying that code.

Worked a fix for related bug 394075 into this patch.

Comment on attachment 335650 [details] [diff] [review] updated, with testcase + if (*src == '%' && *(src+1) == '2') { *(src+1) mighht not be valid. You need to check that src+1 != end. Similar on the next line (an nsACString isn't necessarily nullterminated) I also don't think that appending character-by-character is particularly efficient code... but that probably doesn't matter much. (other code keeps track of how many characters were left unchanged and append them in once call. cf. http://mxr.mozilla.org/mozilla-central/source/xpcom/io/nsEscape.cpp#516) I'd suggest spaces around your + operators but this file doesn't really seem to do that r+sr=biesi if you fix the src+1 thing

> *(src+1) mighht not be valid. so may be: char ch = *(src+2);

Comment on attachment 335671 [details] [diff] [review] Address biesi's comments Approved for 1.8.1.17. a=ss

Comment on attachment 335671 [details] [diff] [review] Address biesi's comments And approved for 1.9.0.2 as well. a=ss

Fix checked in to 1.8.1 and 1.9.0 branches. Will seek retroactive r= from biesi to make sure this is what he wanted

+ if (*(src+2) == 'f' || *(src+***1***) == 'F') + ch = '/'; + else if (*(src+2) == 'e' || *(src+2) == 'E') is the only |+1| on purpose ?

Comment on attachment 335671 [details] [diff] [review] Address biesi's comments + if (*(src+2) == 'f' || *(src+1) == 'F') I agree with georgi, that should be +2 maybe the test should verify the final URL's spec, instead of just verifying that it doesn't contain ..?

This needs to land both on the branch and on the GECKO190_20080827_RELBRANCH

Er, make that both branches (1.8.1 and 1.9) and GECKO190_20080827_RELBRANCH

incremental fix checked into 1.8, 1.9 and _RELBRANCH. Filed bug 452470 for improving the testcase.

Verified on latest build candidates for 20017 and 3.0.2. When I load resource:///..%2F..%2F..%2F I see the contents of these directories: On 20016 Index of file:///Users/user/Desktop/Firefox.app/Contents/MacOS/../../../ Index of file:///home/mozilla/Desktop/firefox/../../../ WinXP/Vista (nothing happens) On 20017 Index of file:///Users/user/Desktop/Firefox.app/Contents/MacOS/ Index of file:///home/mozilla/Desktop/firefox/ Index of file:///C:/Program Files/Mozilla Firefox/ On 3.0.1 Index of file:///Users/user/Desktop/Firefox.app/Contents/MacOS/../../../ Index of file:///home/mozilla/Desktop/firefox/../../../ WinXP/Vista (nothing happens) On 3.0.2 Index of file:///Users/user/Desktop/Firefox.app/Contents/MacOS/ Index of file:///home/mozilla/Desktop/firefox/ Index of file:///C:/Program Files/Mozilla Firefox/

Comment on attachment 335671 [details] [diff] [review] Address biesi's comments a=asac for 1.8.0.15 (with attachment 335741 [details] [diff] [review])

Comment on attachment 335741 [details] [diff] [review] incremental patch on top of what's checked in a=asac for 1.8.0.15 (with attachment 335741 [details] [diff] [review] and iterator backport patch)

biesi, you think this 1.8.0 fix is too ugly for the sake of keeping the other patches the unmodified?

err, here the one that was meant to be attached :)

Comment on attachment 336251 [details] [diff] [review] iterator hack (on top) for 1.8.0 this is for 180 only? fine with me :)

Comment on attachment 336251 [details] [diff] [review] iterator hack (on top) for 1.8.0 a=asac for 1.8.0.15 thanks biesi!

can't we resolve this as fixed, now?

I need someone to check this into mozilla-central to call it "fixed"

Verified for 1.9.0.4 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.4pre) Gecko/2008102304 GranParadiso/3.0.4pre.

This landed before 1.9.1 branched

(In reply to comment #43) > This landed before 1.9.1 branched Adding fixed1.9.1