Summary: Potential Firefox Remote & Local Code Excution 0day → File type confusion vulnerability due to null bytes in URL (encoded as %00)
The comments on that blog, and my testing, seem to indicate that this "exploit" doesn't work remotely (at least not in 2004 due to the resource:// protocol fixes).
Assignee: dveditz → nobody
Component: General → Security
QA Contact: general → firefox
(though it would be interesting to know what troy.js contains, I can't seem to access that file...)
We only pass things to Windows if they don't have an executable extension, but we do that test on an nsAString; see nsLocalFile::IsExecutable in nsLocalFileWin.cpp. On the other hand, nsLocalFile::Launch passes a PRUnichar* pointer to ShellExecute, since that's what the API takes. We should probably either disallow nulls inside nsLocalFile filenames or change IsExecutable to check the same thing that Launch() will use. The latter is probably simpler, while the former would generally be better.
We'll still lie about which helper app we'll use... but at least we'll check the IsExecutable() thing correctly.
Note that nsLocalFile::AppendInternal and nsLocalFile::InitWithPath already do some sanity checks with slashes and whatnot. So we should probably just add similar checks for nulls. And perhaps look at the other nsLocalFile impls too.
Component: Security → Security
Product: Firefox → Core
QA Contact: firefox → toolkit
Luckily file:/// is not available from web pages, and in the limited contexts resource: is allowed (script, stylesheets, images, xbl) we ignore requests going to an external helper app. But if you knew the location of a local file and could somehow get the user to save and execute your .html (an email attachment or other) this might work.
Whiteboard: [sg:investigate] → [sg:low] file:/// not available from web
Flags: blocking1.9? → blocking1.9+
This stops the exploit by checking for an embedded null before creating the local file object. I'm not totally happy with it because it doesn't throw an error page they way an embedded %01 pointing at a non-existing file does, but that's rather minor.
Comment on attachment 272054 [details] [diff] [review] fail if file: has embedded %00 Looks OK to me, but I still wonder whether we should also fix this on the XPCOM level.
Attachment #272054 - Flags: superreview?(bzbarsky) → superreview+
Comment on attachment 272054 [details] [diff] [review] fail if file: has embedded %00 Darin replied by mail: "Seems OK. It is unfortunate that the file URL to file path conversion functions do not share some boiler-plate code that you could patch instead."
Attachment #272054 - Flags: review?(cbiesinger) → review+
Comment on attachment 272054 [details] [diff] [review] fail if file: has embedded %00 Approved for 1.8 branch, a=jay for drivers
Attachment #272054 - Flags: approval188.8.131.52+
Fix checked into trunk and branches
If this is checked-in on trunk, should it still be open?
Marking fixed, reopen if I've made a mistake.
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Priority: -- → P1
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.