Opened 3 years ago

Last modified 3 months ago

#20322 new defect

SafeSEH support for mingw-w64 for Tor Browser on Windows

Reported by: bugzilla Owned by: tbb-team
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: tbb-security, TorBrowserTeam201711, GeorgKoppen201711, tbb-rbm
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor: Sponsor4

Description

Not only SEH, because of www.fuzzysecurity.com/tutorials/expDev/3.html
Even YASM can do it https://www.tortall.net/projects/yasm/manual/html/objfmt-win32-safeseh.html

Child Tickets

Change History (14)

comment:1 Changed 3 years ago by gk

Parent ID: #16010

comment:2 Changed 3 years ago by cypherpunks

As there is no activity on this problem, should we proceed with the public disclosure?

comment:4 Changed 2 years ago by gk

Keywords: TorBrowserTeam201711 GeorgKoppen201711 added
Sponsor: Sponsor4

comment:5 Changed 2 years ago by gk

Priority: MediumVery High

Changing prio to reflect sponsor deadline

comment:6 Changed 2 years ago by gk

Parent ID: #21777
Priority: Very HighMedium

I did some digging and with our GCC-based toolchain this is tricky right now. However, we probably need a clang-based toolchain for ESR 59 anyway due to https://bugzilla.mozilla.org/show_bug.cgi?id=1390583 and it seems we'd get SafeSEH when switching. Thus, it makes no sense to fix this bug right now for the current toolchain. We should get SafeSEH with #21777 being fixed.

comment:7 in reply to:  6 Changed 2 years ago by cypherpunks

Replying to gk:

I did some digging and with our GCC-based toolchain this is tricky right now.

Read comment:3. There is nothing tricky in adding one flag.

Thus, it makes no sense to fix this bug right now for the current toolchain.

Quite the opposite.

There is a very real security benefit to this, mainly because it's so easy for malware to corrupt the SEH chain. Once the SEH chain is corrupted, it's typically very easy to cause an exception, at which point the exception handling machinery will go and dispatch execution to the handlers indicated in the chain. If a handler points into a DLL which doesn't have NO-SEH or SAFESEH, execution will transfer to that address without trouble.

comment:8 Changed 20 months ago by gk

Parent ID: #21777

comment:9 Changed 3 months ago by gk

Keywords: tbb-rbm added

I think we are done here with mingw-w64-clang but we should double-check that.

comment:10 in reply to:  9 ; Changed 3 months ago by gk

Replying to gk:

I think we are done here with mingw-w64-clang but we should double-check that.

It seems the mingw-w64/gcc toolchain is not doing the right thing for 32bit exe/dll files. Thus, fixing this needs #29318 solved first, too.

Last edited 3 months ago by gk (previous) (diff)

comment:11 Changed 3 months ago by gk

Parent ID: #29318

comment:12 Changed 3 months ago by gk

Parent ID: #29318

comment:13 in reply to:  10 Changed 3 months ago by gk

Replying to gk:

Replying to gk:

I think we are done here with mingw-w64-clang but we should double-check that.

It seems the mingw-w64/gcc toolchain is not doing the right thing for 32bit exe/dll files. Thus, fixing this needs #29318 solved first, too.

PTs based on Go need separate treatment. I've opened #31716 for obfs4.

comment:14 Changed 3 months ago by gk

https://docs.microsoft.com/en-us/cpp/build/reference/safeseh-image-has-safe-exception-handlers?redirectedfrom=MSDN&view=vs-2019

/SAFESEH is only valid when linking for x86 targets. /SAFESEH is not supported for platforms that already have the exception handlers noted. For example, on x64 and ARM, all exception handlers are noted in the PDATA. ML64.exe has support for adding annotations that emit SEH information (XDATA and PDATA) into the image, allowing you to unwind through ml64 functions. See MASM for x64 (ml64.exe) for more information.

So, 64bit files PASS in our check script just because they are 64bit ones. I wonder whether we need to do more here, though, given that we don't use ML64.exe.

Note: See TracTickets for help on using tickets.