Opened 3 years ago

Closed 3 years ago

#17895 closed defect (fixed)

Tor Browser Bundle installer subject to DLL hijacking

Reported by: ericlaw Owned by: boklm
Priority: High Milestone:
Component: Applications/Tor Browser Version:
Severity: Major Keywords: tbb-gitian, tbb-security, TorBrowserTeam201604R
Cc: mcs, boklm Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

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/

Child Tickets

Change History (26)

comment:1 Changed 3 years ago by ericlaw

Summary: Tor Browser Bundle subject to DLL hijackingTor Browser Bundle installer subject to DLL hijacking

comment:2 Changed 3 years ago by dcf

Component: Tor bundles/installationTor Browser
Owner: changed from erinn to tbb-team
Version: Tor: 0.2.7.6

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)

comment:3 Changed 3 years ago by gk

Keywords: tbb-gitian tbb-security TorBrowserTeam201512 GeorgKoppen201512 added
Priority: MediumHigh
Severity: CriticalMajor

comment:4 in reply to:  2 Changed 3 years ago by dcf

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, 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."

comment:5 Changed 3 years ago by ericlaw

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.

comment:6 Changed 3 years ago by mcs

Cc: mcs added

comment:7 Changed 3 years ago by gk

Keywords: TorBrowserTeam201601 added; TorBrowserTeam201512 removed

Tickets for Jan 2016.

comment:8 Changed 3 years ago by gk

Keywords: GeorgKoppen201601 added; GeorgKoppen201512 removed

comment:9 Changed 3 years ago by cypherpunks

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

comment:10 Changed 3 years ago by gk

Keywords: TorBrowserTeam201602 added; TorBrowserTeam201601 removed

Putting stuff on the radar for February.

comment:11 Changed 3 years ago by gk

Keywords: GeorgKoppen201602 added; GeorgKoppen201601 removed

Updating my tickets.

comment:12 Changed 3 years ago by gk

Keywords: GeorgKoppen201603 added; GeorgKoppen201602 removed

comment:13 Changed 3 years ago by gk

Keywords: TorBrowserTeam201603 added; TorBrowserTeam201602 removed

comment:14 Changed 3 years ago by gk

Keywords: GeorgKoppen201604 added; GeorgKoppen201603 removed

comment:15 Changed 3 years ago by gk

Keywords: TorBrowserTeam201604 added; TorBrowserTeam201603 removed

comment:16 Changed 3 years ago by anon

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

comment:17 in reply to:  16 Changed 3 years ago by dcf

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.

comment:18 in reply to:  16 ; Changed 3 years ago by gk

Cc: boklm added
Status: newneeds_information

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?

comment:19 in reply to:  18 Changed 3 years ago by boklm

Replying to gk:

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

I can look at this during this week.

comment:20 Changed 3 years ago by gk

Keywords: GeorgKoppen201604 removed
Owner: changed from tbb-team to boklm
Status: needs_informationassigned

Thanks.

comment:21 Changed 3 years ago by anon

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...

comment:22 in reply to:  21 Changed 3 years ago by mcs

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.

comment:23 in reply to:  18 Changed 3 years ago by boklm

Keywords: TorBrowserTeam201604R added; TorBrowserTeam201604 removed
Status: assignedneeds_review

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.

comment:24 Changed 3 years ago by gk

Status: needs_reviewneeds_revision

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

comment:25 Changed 3 years ago by boklm

Status: needs_revisionneeds_review

comment:26 Changed 3 years ago by gk

Resolution: fixed
Status: needs_reviewclosed

Applied to master as commit 186c4e271355646eb4b9faadffaaa7fc0f986a3e, thanks!

Note: See TracTickets for help on using tickets.