Open Bug 599611 Opened 10 years ago Updated 1 year ago

Enable Structured Exception Handling Overwrite Protection (SEHOP) for firefox.exe

Categories

(Firefox :: Installer, enhancement, P5)

x86_64
Windows 7
enhancement

Tracking

()

Tracking Status
blocking2.0 --- -

People

(Reporter: emk, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: sec-want, Whiteboard: [sg:want P2])

Attachments

(2 files)

This will improve malware protection on Windows 7. IE9 enables it by default.
Attachment #478526 - Attachment mime type: application/octet-stream → text/plain
blocking2.0: --- → ?
Feels like something we should do - :rs, opinions?
I only looked at this briefly and am concerned about doing this late in the cycle. If we do this we should should do it as early as possible  and I can throw together a patch within the next 24 hours so the affects can be evaluated in beta 7.
I am concerned about the effects of this on plugins, especially those which use JITs and may actually dynamically generate SEH tables. I'm pretty sure our own JITs don't. We do use static SEH in various places, but I presume that isn't affected by this.

Can somebody provide a MSDN/Microsoft link explaining what exactly the technical effects of this setting are?

In any case, not a blocker.
blocking2.0: ? → -
The technical detail is here:
http://www.uninformed.org/?v=5&a=2&t=txt
This page is linked from MSKB.
http://support.microsoft.com/kb/956607
Note that SEHOP is enabled by default in Windows Server 2008/2008 R2. If it causes any problems, they should have occured also on Win2k8.
Whiteboard: [sg:want P2]
(In reply to Masatoshi Kimura [:emk] from comment #5)
> The technical detail is here:
> http://www.uninformed.org/?v=5&a=2&t=txt
> This page is linked from MSKB.
> http://support.microsoft.com/kb/956607

There are two compatibility issues (described in the 5th paragraph of the first link). The second shouldn't apply to us (the problem should appear with applications that hook ntdll!KiUserExceptionDispatcher and with antivirus software that classify the hooking of that function as malware), the first could harm if "the application invalidates the exception handler chain" legitimately (Cygwin and Skype for example don't work for this reason).

Is it enough if we test SEHOP with most used plugins (Flash, Java, etc.)?
The attached reg files just activate SEHOP for firefox.exe so OOP plugins wouldn't be affected?
(In reply to [Baboo] from comment #8)
> The attached reg files just activate SEHOP for firefox.exe so OOP plugins
> wouldn't be affected?

Right - i don't think this should be done for the plugin container - i believe the plugin authors should test their plugins with SEHOP and set it for their plugin process after verifying compatibility. 

I think we should definitely try to get this turned on for firefox.exe - but of course we need to verify it doesn't break anything.
i've been running Nightly on Windows 7 with SEHOP turned on for about a week now and haven't noticed any issues so far, fwiw.

i suggest we make a patch for the installer that creates these two registry keys (so it will work on 32 and 64 bit Windows 7) and do a full try run, including the performance tests.
Try doesn't use the installer, so it won't show you anything.
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #11)
> Try doesn't use the installer, so it won't show you anything.

oh yeah :( thanks for pointing that out. trying to think of other ways to do this - i guess i could run the whole test suite on a Windows 7 box locally but this is a little painful unless i find a machine to use other than my main dev laptop. i thought about having firefox create the keys itself, but we would then need to restart the process after they've been created. i also couldn't find a good way to determine if SEHOP is actually enabled for a process via procexp etc...
Just a note that i've now been running with SEHOP enabled for about a month and haven't noticed any issues. It was suggested that I work with releng to get the registry keys created on a machine I can run the full test suite including talos/performance tests on - I don't currently have time to push on this but will keep it on my long term to do list, anyone else should feel free to pick this work up :)
Is this on anybody's radar?
See Also: → 1335086
Priority: -- → P5
We set this for the sandboxed process now, dodn't we Bob? I think we can resolve this.
Flags: needinfo?(bobowencode)
It's set for all the sandbox policies yes, but it looks like this bug is about setting the registry to enable it for all processes.
Flags: needinfo?(bobowencode)
Type: defect → enhancement
You need to log in before you can comment on or make changes to this bug.