Closed Bug 479667 Opened 16 years ago Closed 15 years ago

Firefox should use SetProcessDEPPolicy to enable NX on XP SP3

Categories

(Toolkit :: Startup and Profile System, enhancement)

x86
Windows XP
enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9.2a1
Tracking Status
blocking1.9.1 --- .2+
status1.9.1 --- .2-fixed

People

(Reporter: myriachan, Assigned: benjamin)

References

(Blocks 1 open bug)

Details

(Keywords: fixed1.9.0.14, Whiteboard: [sg:want P1])

Attachments

(1 file)

User-Agent: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.0.6) Gecko/2009011913 Firefox/3.0.6 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.0.6) Gecko/2009011913 Firefox/3.0.6 Firefox 3 currently sets the /NXCOMPAT linker flag to enable Data Execution Prevention for itself under Vista. However, this doesn't work under XP, because XP doesn't recognize the /NXCOMPAT header bit. Firefox ought to call SetProcessDEPPolicy to enable Data Execution Prevention on itself if the call exists. It was added to XP 32 in Service Pack 3. It is not possible, barring nasty hacks with spawning a child process, to enable DEP in Firefox on an "OptIn" system66 under XP 32 SP2 or XP 64 SP2. SetProcessDEPPolicy plus /NXCOMPAT covers as much ground as realistically possible. (/NXCOMPAT covers Vista RTM, which didn't have SetProcessDEPPolicy.) Reproducible: Always Steps to Reproduce: 1. Set XP's Data Execution Prevention to "OptIn", the default. 2. Run Firefox under XP 32 SP3. 3. Cause Firefox to execute code from a data page. Actual Results: The code works. Expected Results: An exception should occur. http://msdn.microsoft.com/en-us/library/bb736299(VS.85).aspx It might be wise to continue to allow ATL thunk emulation so that "IE tab" can still work with ActiveX plugins for IE that don't like NX.
We should get this done for 1.9.1.x; it won't help people on SP2, but as mentioned in comment #0, we should cover as much ground as possible.
Status: UNCONFIRMED → NEW
blocking1.9.1: --- → ?
Ever confirmed: true
I really mean "badly wanted for 1.9.1.2" with my nomination, but we don't have wanted-1.9.1 or wanted-1.9.1.2 or wanted-1.9.1x or blocking-any-of-those, so this is as close as I could make it.
blocking1.9.1: ? → ---
Flags: blocking1.9.1.1?
blocking 1.9.1 maybe? Well someone will sort it out I hope :)
blocking1.9.1: --- → ?
Component: Security → Startup and Profile System
Product: Firefox → Toolkit
QA Contact: firefox → startup
Assignee: nobody → benjamin
This dynamically loads and calls SetProcessDEPPolicy if it is available.
Attachment #388698 - Flags: review?
Attachment #388698 - Flags: review? → review?(jmathies)
I'll concur with gal here. Getting heap DEP working on as many windows variants as possible should be very high priority. (Also consider workaround N+1: if you can't convince the loader and heap-setup code to do it, you might still be in a condition that VirtualProtect could force it page-by-page inside jemalloc. Maybe? Worth considering?)
Comment on attachment 388698 [details] [diff] [review] Call SetProcessDEPPolicy if available, rev. 1 [Checkin: Comment 10 & 13] Code looks fine. A comment explaining what this is doing or a reference to this bug# might be a good idea.
Attachment #388698 - Flags: review?(jmathies) → review+
blocking1.9.1: ? → needed
Flags: wanted1.9.0.x+
Flags: blocking1.9.1.1?
Flags: blocking1.9.0.13?
Whiteboard: [sg:want P1]
Does this actually enable the ATL thunk workaround at the original comment suggested?
(In reply to comment #8) > Does this actually enable the ATL thunk workaround at the original comment > suggested? I don't think PROCESS_DEP_DISABLE_ATL_THUNK_EMULATION is required because /NXCOMPAT also disables the ATL thunk emulation. http://support.microsoft.com/kb/948468
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Attachment #388698 - Flags: approval1.9.1.2?
I think we should definitively take this for 3.5.2. This would have stopped/disarmed the issue published 3 days ago.
blocking1.9.1: needed → ?
Attachment #388698 - Flags: approval1.9.1.2? → approval1.9.1.2+
Comment on attachment 388698 [details] [diff] [review] Call SetProcessDEPPolicy if available, rev. 1 [Checkin: Comment 10 & 13] a1912=beltzner
blocking1.9.1: ? → .2+
Flags: blocking1.9.0.13? → blocking1.9.0.13+
Keywords: fixed1.9.1
What is the best/simplest way for QA to verify this bug for Firefox 3.5.2? (I find the original STR to be a bit too technical -- steps to perform step 1 and a test URL for step 3 would help...)
I don't think there is a way to test this without writing a test extension or finding an exploitable JIT bug.
You could back out the fix for the heap spray attack and then use the shell code in that bug to test this.
Attachment #388698 - Flags: approval1.9.0.14+
Comment on attachment 388698 [details] [diff] [review] Call SetProcessDEPPolicy if available, rev. 1 [Checkin: Comment 10 & 13] Approved for 1.9.0.14, a=dveditz for release-drivers
Keywords: checkin-needed
Here an example of how the bug can be reproduced (for anyone interested, even though the bug is marked 'resolved'): 1) Start Firefox. 2) And now (after Firefox is initialized, which is important if we are to test whether Firefox calls SetProcessDEPPolicy) attach the OllyDbg debugger to the Firefox process (this should automatically pause the process). 3) Insert some valid assembly code (e.g. a simple 'nop' will do) in the .data section (use View -> Memory to find the .data section). 4) Set the EIP register to the starting offset of the code inserted at step 3. (View -> CPU. Then press CTRL+G or right-click -> Go to -> Expression. There insert the offset. Then CTRL+NUMPAD* or right-click -> New origin here.) 5) Press F8 or Debug -> Step over. 6) If OllyDbg displays 'Access violation when executing ...' in the status bar, the bug is indeed fixed. Using these steps, the bug should not be reproducable with Firefox 3.5.2. Hope someone finds this interesting/useful.
Attachment #388698 - Attachment description: Call SetProcessDEPPolicy if available, rev. 1 → Call SetProcessDEPPolicy if available, rev. 1 [Checkin: Comment 10 & 13]
Whiteboard: [sg:want P1] → [c-n: to cvs=1.9.0] [sg:want P1]
Target Milestone: --- → mozilla1.9.2a1
Version: unspecified → Trunk
Checking in toolkit/xre/nsAppRunner.cpp; /cvsroot/mozilla/toolkit/xre/nsAppRunner.cpp,v <-- nsAppRunner.cpp new revision: 1.219; previous revision: 1.218 done
Whiteboard: [c-n: to cvs=1.9.0] [sg:want P1] → [sg:want P1]
Depends on: 515718
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: