The default bug view has changed. See this FAQ.

Investigate VC8 Compile Flags

RESOLVED FIXED

Status

()

Core
Build Config
P1
normal
RESOLVED FIXED
10 years ago
8 years ago

People

(Reporter: Mike Schroepfer, Assigned: bsmedberg)

Tracking

(Blocks: 1 bug)

Trunk
x86
Windows XP
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

10 years ago
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?
(Reporter)

Updated

10 years ago
Component: General → Build Config
Product: Firefox → Core
QA Contact: general → build-config
(Reporter)

Comment 1

10 years ago
Note it will be necessary to setup a test tinderbox to experiment with these flags.   
(Assignee)

Updated

10 years ago
Assignee: nobody → benjamin
Priority: -- → P1
(Assignee)

Comment 2

10 years ago
Created attachment 272527 [details] [diff] [review]
NXCompat and SAFESEH, rev. 1

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+
(Assignee)

Comment 3

10 years ago
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
Last Resolved: 10 years ago
Resolution: --- → FIXED
(Assignee)

Updated

10 years ago
Blocks: 392631

Updated

10 years ago
Duplicate of this bug: 392631
Flags: in-testsuite-

Comment 5

10 years ago
It looks like we need to add these flags to FF 2 builds.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 6

10 years ago
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.
Depends on: 418613

Comment 7

9 years ago
we should be able to post process using a newer editbin if someone really cares.

Comment 8

9 years ago
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
Last Resolved: 10 years ago9 years ago
Resolution: --- → FIXED

Updated

9 years ago
Depends on: 405523
(Assignee)

Updated

9 years ago
Depends on: 427382

Updated

8 years ago
Blocks: 504250
You need to log in before you can comment on or make changes to this bug.