Tor Browser Bundle installer subject to DLL hijacking

torbrowser-install-5.0.4.exe is vulnerable to DLL hijacking.

Create, e.g. shfolder.dll with a malicious DLL main and observe it runs when the tor installer is executed from the same downloads folder.

http://textslashplain.com/2015/12/18/dll-hijacking-just-wont-die/

Trac:
Username: ericlaw

added TorBrowserTeam201604R component::applications/tor browser owner::boklm priority::high reporter::ericlaw resolution::fixed severity::major status::closed tbb-gitian tbb-security type::defect labels

Trac:
Username: ericlaw
Summary: Tor Browser Bundle subject to DLL hijacking to Tor Browser Bundle installer subject to DLL hijacking

According to the blog post, we just need to update NSIS to version 2.49.

It seems the DLL hijacking fix was actually in version 2.47 (released 08 December 2015):

http://sourceforge.net/projects/nsis/files/NSIS%202/2.47/RELEASE.html/view

• LoadLibrary security hardening to prevent dll hijacking (patch #1125)

Trac:
Owner: erinn to tbb-team
Version: Tor: 0.2.7.6 to N/A
Component: Tor bundles/installation to Tor Browser

Trac:
Severity: Critical to Major
Keywords: N/A deleted, GeorgKoppen201512, tbb-gitian, TorBrowserTeam201512, tbb-security added
Priority: Medium to High

Replying to dcf:

According to the blog post, we just need to update NSIS to version 2.49.

It seems the DLL hijacking fix was actually in version 2.47 (released 08 December 2015):

In the longer term we want to upgrade to the NSIS 3.0 series, because it will enable us to use more languages in the installer: see #13469 (moved), especially comment:6:ticket:13469.

But according to http://nsis.sourceforge.net/Main_Page, the current version 3.0b2 was released 04 August 2015, so it probably doesn't have the DLL hijacking fix. Eric's blog post says: "The v3 beta branch doesn’t appear to have the fix, yet."

The NSIS team says the v3 branch should get the fix "this week".

You need 2.49 (rather than 2.47) because the earlier build introduced a regression which was fixed in 2.49.

Trac:
Username: ericlaw

Trac:
Cc: N/A to mcs

Tickets for Jan 2016.

Trac:
Keywords: TorBrowserTeam201512 deleted, TorBrowserTeam201601 added

Trac:
Keywords: GeorgKoppen201512 deleted, GeorgKoppen201601 added

Need to update to 2.50 as that is the latest and also includes some minor fixes relating to this.

Putting stuff on the radar for February.

Trac:
Keywords: TorBrowserTeam201601 deleted, TorBrowserTeam201602 added

Updating my tickets.

Trac:
Keywords: GeorgKoppen201601 deleted, GeorgKoppen201602 added

Trac:
Keywords: GeorgKoppen201602 deleted, GeorgKoppen201603 added

Trac:
Keywords: TorBrowserTeam201602 deleted, TorBrowserTeam201603 added

Trac:
Keywords: GeorgKoppen201603 deleted, GeorgKoppen201604 added

Trac:
Keywords: TorBrowserTeam201603 deleted, TorBrowserTeam201604 added

Is this blocked on upstream NSIS 2.49, NSIS 3.x update, lacking dev time, or something else?

Trac:
Username: anon
Reviewer: N/A to N/A

Replying to anon:

Is this blocked on upstream NSIS 2.49, NSIS 3.x update, lacking dev time, or something else?

I think it's blocked on dev time.

If you're a developer, you can try and make a patch. Here is the guide to working on the Tor Browser build system: https://trac.torproject.org/projects/tor/wiki/doc/TorBrowser/Hacking.

It looks like the build system is using the nsis package from Ubuntu precise, so you might have to find a way to instead use a backported more recent version, or build from source.

Replying to anon:

Is this blocked on upstream NSIS 2.49, NSIS 3.x update, lacking dev time, or something else?

Lack of dev time. We have been mostly busy with getting Tor Browser switched to Firefox ESR45 and we restructured our Tor Browser team (we are a bit smaller now and I am responsible for the team management stuff, now, too).

What we need here is:

1. Cross-compiling NSIS
2. Making sure the resulting .exe files are still bit-by-bit reproducible
3. Making sure that these files are still working on all supported Windows versions (XP - 10)
4. Making sure stripping the authenticode signature is still reproducible

Thanks for pointing out that this is not done within 5 minutes.

That said I agree with this being an important issue and I'd like to have this fixed rather sooner than later. Ideally, before the 6.0 gets stable. I looked a bit at 1) this morning with NSIS 2.51 but already that step is failing badly for me: I took the cross-compiler we generate during our Windows build and followed the sparse cross-compile documentation. Starting the build just broke with

sh: 1: Syntax error: "(" unexepected

while compiling advsplash.c. I then tried to get the necessary help by looking at the way Debian builds NSIS but that did not work either for me.

boklm: Is this something you would have time to look into?

Trac:
Status: new to needs_information
Cc: mcs to mcs, boklm

Replying to gk:

boklm: Is this something you would have time to look into?

I can look at this during this week.

Thanks.

Trac:
Owner: tbb-team to boklm
Status: needs_information to assigned
Keywords: GeorgKoppen201604 deleted, N/A added

Thank you for the detailed reply!

I have not dug into the details of Tor Browser GUI -> Open Menu -> Options -> Advanced -> Update [auto update enabled by default].

Does the Firefox update process use this installer wrapper? perhaps with special parameters? Or is this upgrade path completely unaffected...

Trac:
Username: anon

Replying to anon:

Does the Firefox update process use this installer wrapper? perhaps with special parameters? Or is this upgrade path completely unaffected...

The update process does not use an NSIS-based installer.

Replying to gk:

What we need here is:

1. Cross-compiling NSIS

The branch bug_17895 in my user repo is doing that: https://gitweb.torproject.org/user/boklm/tor-browser-bundle.git/commit/?h=bug_17895&id=ed474700d85d135fa1e1bf6ae358a9c781d8dac6

To fix the build issues, we are using the patches from the Debian package.

2. Making sure the resulting .exe files are still bit-by-bit reproducible

I checked that re-bundling results in the same .exe file. I did not check yet that it is also the case after a make clean-utils, I will try it tomorrow.

3. Making sure that these files are still working on all supported Windows versions (XP - 10)
4. Making sure stripping the authenticode signature is still reproducible

I did not check that yet.

Trac:
Keywords: TorBrowserTeam201604 deleted, TorBrowserTeam201604R added
Status: assigned to needs_review

Thanks! This looks good to me. Some nits:

1. in mkbundle-windows.sh look at how we treat binutils, gcclibs and all the others: we should rebuild the utils if there is a new NSIS version, too. Additionally, we should refresh the link as well in case we are skipping the utilities build to make sure we are always use the correct version.

2. We should verify the packages in verify-tags.sh as well.

3. You could add the NSIS packages to versions.alpha, too

Trac:
Status: needs_review to needs_revision

I added the branch bug_17895_v2 with the changes you mentioned: https://gitweb.torproject.org/user/boklm/tor-browser-bundle.git/commit/?h=bug_17895_v2&id=a03e65a51306412173d59432e7df7bc7c8884511

Trac:
Status: needs_revision to needs_review

Applied to master as commit 186c4e271355646eb4b9faadffaaa7fc0f986a3e, thanks!

Trac:
Resolution: N/A to fixed
Status: needs_review to closed

closed

moved to tpo/applications/tor-browser#17895 (closed)