Closed Bug 1021214 Opened 7 years ago Closed 2 years ago

Run BinScope on all DLLs and EXEs that we build

Categories

(Firefox Build System :: General, defect)

All
Windows 7
defect
Not set
normal

Tracking

(firefox-esr68 wontfix, firefox74 wontfix, firefox75 wontfix, firefox76 fixed)

RESOLVED FIXED
mozilla76
Tracking Status
firefox-esr68 --- wontfix
firefox74 --- wontfix
firefox75 --- wontfix
firefox76 --- fixed

People

(Reporter: TimAbraldes, Assigned: glandium)

References

Details

(Keywords: sec-want, Whiteboard: [sg:want P2][post-critsmash-triage][adv-main76-][adv-ESR68.8-])

Attachments

(1 file, 1 obsolete file)

+++ This bug was initially created as a clone of Bug #642243 +++

Bug 642243 was intended to prevent us from running into issues like bug 641630, but it only added support for us running BinScope against firefox.exe and plugin-container.exe. As a result, bug 641630 could regress without us knowing (meaning that bug 642243 hasn't accomplished its goal).

Ideally we'd run BinScope against every DLL and EXE that we build.

I'm copying the [sg:want P2] whiteboard flag from bug 644243 to this bug because presumably the "want" hasn't gone away.

Also I'm marking this as hidden because I'm also filing a couple dependent bugs that will be marked hidden as well
Depends on: 1021215
> I'm copying the [sg:want P2] whiteboard flag from bug 644243 to this bug
> because presumably the "want" hasn't gone away.

That should say bug 642243
Depends on: 642243
It seems a little bit silly to have this be a hidden bug and then say exactly what this bug is about on bug 642243.
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #2)
> It seems a little bit silly to have this be a hidden bug and then say
> exactly what this bug is about on bug 642243.

True. I probably should have waited to comment on bug 642243 until after this bug was resolved.
Attached patch Patch v1 (WIP) (obsolete) — Splinter Review
Something like this maybe? I don't know Makefiles so this is just me fiddling around.

Note that we'll need to implement some kind of whitelist; either for specific DLLs or EXEs, or for specific BinScope failures
Group: core-security → core-security-release
Keywords: sec-want
I'll do this along with bug 1236356.
Assignee: nobody → birunthan
Status: NEW → ASSIGNED
Does this really need to be a hidden bug ?
(In reply to Ian Melven :imelven from comment #6)
> Does this really need to be a hidden bug ?

I don't really remember enough context on this to have an opinion one way or the other. It's up to sec team
Assignee: birunthan → nobody
Status: ASSIGNED → NEW
Product: Core → Firefox Build System
Duplicate of this bug: 1412918
Attachment #8437286 - Attachment is obsolete: true
Assignee: nobody → mh+mozilla

This needs a few adjustments to the autobinscope script because running
binscope currently creates an HTML file in the binscope directory, and
when multiple binscopes run at the same time (which happens during the
build with the changes to run it on all executables and libraries), all
but one fail to open the HTML file for write access.

So add a flag to create that file in a temporary directory.

While here, remove log_file_path, which hasn't been used since
bug 1448306.

Blocks: 1618782

For the record, this was landed and subsequently backed out. It turns out this fails on 32-bits PGO builds, both generate and use.

z:\task_1584670060\workspace\obj-build\toolkit\crashreporter\injector\breakpadinjector.dll: error BA2013: breakpadinjector.dll is a C or C++ binary that does not initialize the stack protector. The stack protector (/GS) is a security feature of the compiler which makes it more difficult to exploit stack buffer overflow memory corruption vulnerabilities. The stack protector requires access to entropy in order to be effective, which means a binary must initialize a random number generator at startup, by calling __security_init_cookie() as close to the binary's entry point as possible. Failing to do so will result in spurious buffer overflow detections on the part of the stack protector. To resolve this issue, use the default entry point provided by the C runtime, which will make this call for you, or call __security_init_cookie() manually in your custom entry point.

But now that bug 1620166 landed, this could reland without breaking anything because the PGO builds don't run autobinscope anymore, but I'd rather wait for subsequent changes (such as bug 1450088 and bug 1618782) to be ready before doing so (in case 1620166 is backed out again)

Blocks: 1450088
Flags: qe-verify-
Whiteboard: [sg:want P2] → [sg:want P2][post-critsmash-triage]
Whiteboard: [sg:want P2][post-critsmash-triage] → [sg:want P2][post-critsmash-triage][adv-main76-][adv-ESR68.8-]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.