Closed Bug 805173 Opened 9 years ago Closed 5 years ago
Consider enabling Windows' Heap
Enable Termination On Corruption for browser and plugin-container
59 bytes, text/x-review-board-request
Even though Firefox uses the jemalloc allocator, enabling HeapEnableTerminationOnCorruption for the default process heap could detect problems from system libraries or third-party plugins that use Microsoft's malloc() or HeapAlloc(). Win64 enables HeapEnableTerminationOnCorruption by default for 64-bit applications, but not for 32-bit applications. The "Reversing on Windows" security blog demonstrates that Chrome and IE enable this mitigation feature on Windows, but Firefox does not: http://reversingonwindows.blogspot.com/2012/05/test-for-termination-on-heap-corruption.html WebKit enables HeapEnableTerminationOnCorruption here: https://code.google.com/searchframe#OAMlx_jo-ck/src/third_party/WebKit/Source/WebKit2/WebProcess/WebKitMain.cpp&l=116 Firefox could enable HeapEnableTerminationOnCorruption for the browser and plugin-container processes here: https://mxr.mozilla.org/mozilla-central/source/toolkit/xre/nsAppRunner.cpp#4038
With the sandbox, this mitigation is actually enabled for content processes (all kinds), so this is really just applicable to the chrome process.
Submitted a review attempting to implement this. I'm not familiar with this area of the code, so I attempted to place it at the location suggested originally.
This looks good to me, but I'm not familiar with Windows APIs in general. Redirecting to Matt for that. Also, I imagine you should get review from a toolkit/browser peer.
Attachment #8853397 - Flags: review?(dkeeler) → review?(mhowell)
Thanks for the redirect! (For context, I'd r?'d you since you were the triage owner, which in my head seemed relevant :-))
Comment on attachment 8853397 [details] Bug 805173 - Enable HeapEnableTerminationOnCorruption for chrome processes on Windows. https://reviewboard.mozilla.org/r/125508/#review128180
Attachment #8853397 - Flags: review?(tom) → review+
I'm not sure my review counts for much but LGTM.
Comment on attachment 8853397 [details] Bug 805173 - Enable HeapEnableTerminationOnCorruption for chrome processes on Windows. https://reviewboard.mozilla.org/r/125508/#review128202 r+ with comments. ::: toolkit/xre/nsAppRunner.cpp:3718 (Diff revision 1) > } > #endif /* DEBUG */ > > +#if defined(XP_WIN) > + // Enable the HeapEnableTerminationOnCorruption exploit mitigation. We ignore > + // the return code because this function always succeeds on recent Windows, I don't see a guarantee that this function always succeeds, but there's no useful handling we can do if it fails, so oh well. ::: toolkit/xre/nsAppRunner.cpp:3719 (Diff revision 1) > #endif /* DEBUG */ > > +#if defined(XP_WIN) > + // Enable the HeapEnableTerminationOnCorruption exploit mitigation. We ignore > + // the return code because this function always succeeds on recent Windows, > + // and always fails on Windows older than XP SP3. MSDN documents that the function returns success (but has no effect) even on < XP SP3. We're not really interested in the behavior of < XP SP3, but I don't want the comment to be misleading.
Attachment #8853397 - Flags: review?(mhowell) → review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/50413067c552 Enable HeapEnableTerminationOnCorruption for chrome processes on Windows. r=mhowell,tjr
You need to log in before you can comment on or make changes to this bug.