Closed Bug 368854 Opened 18 years ago Closed 16 years ago

Investigate VC8 Compile Flags

Categories

(Firefox Build System :: General, defect, P1)

x86
Windows XP
defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: mtschrep, Assigned: benjamin)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Now that we have upgraded to VC8 we should investigate whether to use any of the new compiler flags for performance or security.  Of interest:

Compile with /GS (stack defenses)
Link with /dynamicbase (ASLR in Vista), /NXCompat (NX support) and /SafeSEH (exception handler defenses)

From email discussion:

> Compile with /GS (stack defenses)

This is the default (which we don't override) for VC8, so it should be in force already.

> Link with /dynamicbase (ASLR in Vista), /NXCompat (NX support) and 
> /SafeSEH (exception handler defenses)

I cannot find documentation for /dynamicbase

FF3 should handle NXCompat without problem.

For Moz2, Tamarin has a JIT engine and is going to need special handling of generated executable code.

The /SafeSEH docs confuse me. What use is "a table of the image's safe exception handlers"? Is this used in conjunction with NXCompat, or somehow else? We use little (no) exception handling from within Moz.

> You want to restrict the exception handlers to prevent an exploit from 
> choosing where to jump (into the attackers malicious code, wherever 
> that may be or else into a legit function that does something to the 
> attackers
> advantage.)

Yes, but... does the /safeseh flag turn this protection on, somehow, or only provide the table of safe EH locations?

If we're not using exceptions at all (which I believe is correct), can we just mark the binary as not safe for exception handling at all?
Component: General → Build Config
Product: Firefox → Core
QA Contact: general → build-config
Note it will be necessary to setup a test tinderbox to experiment with these flags.   
Assignee: nobody → benjamin
Priority: -- → P1
This adds NXCompat and SafeSEH, which are both at least documented on microsoft.com.

-dynamicBase was added in VC8SP1 and is undocumented. There are some blog entries on msdn.com that give basic descriptions of it, and it sounds like it only affects the base address of threads that are not the main thread. This isn't especially useful for Moz, since the main thread is where we get most of our security exposure. Implementing it would be hard because the compiler version number didn't rev with SP1 so I'm going to punt on it unless I hear otherwise.
Attachment #272527 - Flags: review?(ted.mielczarek)
Attachment #272527 - Flags: review?(ted.mielczarek) → review+
Fixed on trunk. Window, I'm going to close this bug. If you think we really need -dynamicBase, please open another bug for that specifically.
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Blocks: 392631
Flags: in-testsuite-
It looks like we need to add these flags to FF 2 builds.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
We cannot add these flags to FF2 because FF2 is built with MSVC6 which does not support any of these flags, and changing that is basically impossible.
we should be able to post process using a newer editbin if someone really cares.
C:\home\Desktop\firefox-3.0b4pre.en-US.win32\firefox>dumpbin /headers    firefox.exe
Microsoft (R) COFF/PE Dumper Version 8.00.50727.762
Copyright (C) Microsoft Corporation.  All rights reserved.

Dump of file firefox.exe
...
OPTIONAL HEADER VALUES
...
             100 DLL characteristics
                   NX compatible

this is fixed. aslr is bug 405523. if someone wants to worry about other products/versions, they should file new bugs. for the branch, that'd involve installing

editbin /?
Microsoft (R) COFF/PE Editor Version 8.00.50727.762

somewhere *outside* the default path, and adding a post script which performs
%editbin% /NXCOMPAT *.exe *.dll plugins\*.dll components\*.dll
Status: REOPENED → RESOLVED
Closed: 17 years ago16 years ago
Resolution: --- → FIXED
Depends on: 405523
Depends on: npturnmed
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: