Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#13594 closed defect (fixed)

Tor Browser Bundle 4.0: updater fails on Windows

Reported by: marc Owned by: mcs
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Keywords: TorBrowserTeam201411, MikePerry201411R, tbb-4.5-alpha, tbb-4.0-backport
Cc: brade, mcs, gk, mikeperry Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Tor Browser Updates works fine on Windows 7 32-bit, but fails on Win XP sp3 32 bit

Steps to reproduce:

1) Download and install "torbrowser-install-4.0-alpha-3_es-ES.exe" or "torbrowser-install-4.0-alpha-3_en-US.exe" in a clean directory from
https://archive.torproject.org/tor-package-archive/torbrowser/4.0-alpha-3/

2) Run installed Tor Browser from created shortcut

4) Go to Help ... About Tor Browser and press "Check for updates"

5) When download finishes, click on restart to apply update

Actual results:

Tor Browser won't update and also will not start anymore.

Update files are leaved on
\Tor Browser\Browser\TorBrowser\Data\Browser\Caches\firefox\updates\0

Expected Results:

Tor Browser should update from 4.0-aplha-3 to 4.0 stable as it does on Windows 7

Workaround:
Remove update files from "...\Tor Browser\Browser\TorBrowser\Data\Browser\Caches\firefox" to let non-updated TBB run again.

Child Tickets

Change History (28)

comment:1 Changed 5 years ago by cypherpunks

Can you to grab Tor Browser\Browser\TorBrowser\Data\Browser\Caches\firefox\updates\ folder by compressor (like 7zip or something) and attach to this ticket or to share it by some file-sharing site?

comment:2 in reply to:  1 Changed 5 years ago by marc

Replying to cypherpunks:

Can you to grab Tor Browser\Browser\TorBrowser\Data\Browser\Caches\firefox\updates\ folder by compressor (like 7zip or something) and attach to this ticket or to share it by some file-sharing site?

uploaded here:
http://rghost.net/58766364

comment:3 Changed 5 years ago by marc

Summary: Tor Browser Bundle 4.0: updater fails on Windows XPTor Browser Bundle 4.0: updater fails on Intel Pentium 4

comment:4 Changed 5 years ago by marc

Now I've tested this issue on a VirtualBox Windows 7 system (no sp1) and TBB update also fails on this Win 7.
I have other mozilla browsers (seamonkey, firefox and pale moon) on this XP system and updater always works fine.

Edit: Removed non relevant info. about Pentium CPU's.

Last edited 5 years ago by marc (previous) (diff)

comment:5 Changed 5 years ago by cypherpunks

So, I changed title for this bug report to "Tor Browser Bundle 4.0: updater fails on Intel Pentium 4".

Every CPU should be affected, bug 100% reproducible.

During startup firefox checks for updater.exe and start it if can. Problem with libssp-0.dll dynamically linked to updater.exe. Loader tries to search dll:

C:\Documents and Settings\user\Desktop\Tor Browser\Browser\TorBrowser\Data\Browser\Caches\firefox\updates\0\libssp-0.dll NOT FOUND
C:\WINDOWS\system32\libssp-0.dll NOT FOUND
C:\WINDOWS\system\libssp-0.dll NOT FOUND
C:\WINDOWS\libssp-0.dll NOT FOUND
C:\Program Files\Mozilla Firefox\libssp-0.dll NOT FOUND ( W.T.H ??!!!)
C:\WINDOWS\system32\libssp-0.dll NOT FOUND
C:\WINDOWS\libssp-0.dll NOT FOUND
C:\WINDOWS\System32\Wbem\libssp-0.dll NOT FOUND

But fails with every tried path, then updater terminates without any message box. Dll stored in c:\Documents and Settings\user\Desktop\Tor Browser\Browser\libssp-0.dll but loader ignores that path (it tries to load from C:\Program Files\Mozilla Firefox\libssp-0.dll instead).
The same problem with dynamically linked msvcr100.dll, loader can't handle proper path and tries to load it from C:\Program Files\Mozilla Firefox (successfully if Firefox installed).

comment:6 Changed 5 years ago by cypherpunks

firefox checks for updater.exe

For clear: updater placed in and started from Tor Browser\Browser\TorBrowser\Data\Browser\Caches\firefox\updates\0 folder.

comment:7 Changed 5 years ago by marc

Summary: Tor Browser Bundle 4.0: updater fails on Intel Pentium 4Tor Browser Bundle 4.0: updater fails on Windows

comment:8 in reply to:  5 Changed 5 years ago by marc

Replying to cypherpunks:

So, I changed title for this bug report to "Tor Browser Bundle 4.0: updater fails on Intel Pentium 4".

Every CPU should be affected, bug 100% reproducible.

Ok, title changed again. :)

During startup firefox checks for updater.exe and start it if can. Problem with libssp-0.dll dynamically linked to updater.exe. Loader tries to search dll:

C:\Documents and Settings\user\Desktop\Tor Browser\Browser\TorBrowser\Data\Browser\Caches\firefox\updates\0\libssp-0.dll NOT FOUND
...

This makes sense.
I've found that the Win 7 machine has "libssp-0.dll" on Pidgin install folder.

The same problem with dynamically linked msvcr100.dll, loader can't handle proper path and tries to load it from C:\Program Files\Mozilla Firefox (successfully if Firefox installed).

My Win 7 has "msvcr100.dll" on System32 folder.
Probably that's why TBB is updating on Win 7 machine and not the XP or Virtual Win 7 (both has no pidgin installed and no 'libssp-0.dll', also no Firefox Installed).

Thanks!

comment:9 Changed 5 years ago by cypherpunks

Firefox removing the current directory from the standard search path by calling SetDllDirectory with an empty string, and In Windows, the SetDllDirectory API affects how child processes load implicitly-linked DLLs.

comment:10 Changed 5 years ago by cypherpunks

Nothing

Last edited 5 years ago by cypherpunks (previous) (diff)

comment:11 Changed 5 years ago by cypherpunks

and no ways to bypass it.

Except to rename firefox.exe to something different.

Version 0, edited 5 years ago by cypherpunks (next)

comment:12 Changed 5 years ago by cypherpunks

Nothing

Last edited 5 years ago by cypherpunks (previous) (diff)

comment:13 Changed 5 years ago by cypherpunks

Nothing

Last edited 5 years ago by cypherpunks (previous) (diff)

comment:14 Changed 5 years ago by mcs

Cc: brade mcs gk added

I have not tested it yet, but would copying libssp-0.dll and msvcr100.dll into the updates\0 directory (next to the copy of updater.exe) fix this problem?

comment:15 in reply to:  14 ; Changed 5 years ago by mcs

Replying to mcs:

I have not tested it yet, but would copying libssp-0.dll and msvcr100.dll into the updates\0 directory (next to the copy of updater.exe) fix this problem?

Answering my own question... I did test this solution by manually copying the two needed DLLs into the updates\0 directory right before I clicked the "Restart To Update" button. It worked. The downside of this solution is that we would need to hard-code the names of those two DLLs into the browser, here:

http://mxr.mozilla.org/mozilla-esr31/source/toolkit/xre/nsUpdateDriver.cpp#338

which means that if, in the future, updater.exe depends on another DLL or if the name of one of the DLLs is changed, things will break again.

I have not looked yet, but Mozilla must link their updater.exe in a special way to avoid external dependencies. That might be the best solution, but it is probably more difficult to implement.

comment:16 in reply to:  15 ; Changed 5 years ago by gk

Replying to mcs:

Replying to mcs:

I have not tested it yet, but would copying libssp-0.dll and msvcr100.dll into the updates\0 directory (next to the copy of updater.exe) fix this problem?

Answering my own question... I did test this solution by manually copying the two needed DLLs into the updates\0 directory right before I clicked the "Restart To Update" button. It worked. The downside of this solution is that we would need to hard-code the names of those two DLLs into the browser, here:

http://mxr.mozilla.org/mozilla-esr31/source/toolkit/xre/nsUpdateDriver.cpp#338

which means that if, in the future, updater.exe depends on another DLL or if the name of one of the DLLs is changed, things will break again.

Hmm... yes that is unfortunate, indeed.

I have not looked yet, but Mozilla must link their updater.exe in a special way to avoid external dependencies. That might be the best solution, but it is probably more difficult to implement.

That is tricky, yes, as we e.g. for XP users link against msvcr100 and our hardening plays a role here, too. If I understand comment:9 and comment:12 correctly then that might be a good solution we could try.

comment:17 in reply to:  16 ; Changed 5 years ago by mcs

Replying to gk:

That is tricky, yes, as we e.g. for XP users link against msvcr100 and our hardening plays a role here, too. If I understand comment:9 and comment:12 correctly then that might be a good solution we could try.

I did some more digging to figure out why the updater succeeds on some Windows systems.
On one of our Win7 systems, when starting updater.exe, it looks like a copy of libssp-0.dll from Browser\TorBrowser\Tor\ is used. This happens because Tor Launcher has added that directory to the PATH:

https://gitweb.torproject.org/tor-launcher.git/blob/HEAD:/src/components/tl-process.js#l389

And because there is a copy of msvcr100.dll in C:\Windows\System32\, updater.exe can be loaded.

My conclusion the msvcr100.dll is the real problem (I will need to test again on WinXP to be sure).

Do you know if all Win7 systems have a copy of msvcr100.dll in their system directory? If so, then probably only WinXP (and possibly Vista) users are affected by this bug.

Although comment:12 has been "deleted" I think its contents are still relevant. One potential solution would be to ensure that the PATH contains the Browser\ directory (since it contains both libssp-0.dll and msvcr100.dll). This would only need to be done when starting the updater.

comment:18 in reply to:  17 Changed 5 years ago by gk

comment:12 was:

and no ways to bypass it.


It changes PATH environment variable.
What for PATH in Tor Browser? What if to change call of SanitizeEnvironmentVariables (or directly SanitizeEnvironmentVariables), to clear PATH and to fill it by:

  1. The directory from which Tor Browser loaded.
  2. The system directory. Use the GetSystemDirectory function to get the path of this directory.
  3. The Windows directory. Use the GetWindowsDirectory function to get the path of this directory.

It allows to protect against injected paths and fixes updater.exe without extra voodoo or copying of need dlls to need dir.

comment:19 in reply to:  17 Changed 5 years ago by gk

Replying to mcs:

Replying to gk:
Do you know if all Win7 systems have a copy of msvcr100.dll in their system directory? If so, then probably only WinXP (and possibly Vista) users are affected by this bug.

No idea.

Although comment:12 has been "deleted" I think its contents are still relevant. One potential solution would be to ensure that the PATH contains the Browser\ directory (since it contains both libssp-0.dll and msvcr100.dll). This would only need to be done when starting the updater.

Sounds good to me.

comment:20 Changed 5 years ago by cypherpunks

Do you know if all Win7 systems have a copy of msvcr100.dll in their system directory? If so, then probably only WinXP (and possibly Vista) users are affected by this bug.

msvcr100.dll is part of standalone Visual C++ redistribution package. Redistributable Package installed by Visual Studio or by a 3rd party softwares, every day used computers with windows7 usually have to install it. Except not every newer redistributable package versions contains it but Microsoft Visual C++ 2010 SP1 Redistributable Package.

Last edited 5 years ago by cypherpunks (previous) (diff)

comment:21 Changed 5 years ago by mikeperry

Keywords: TorBrowserTeam201411 added

comment:22 Changed 5 years ago by mcs

Keywords: MikePerry201410R added
Owner: changed from tbb-team to mcs
Status: newassigned

A fix is available here:

https://gitweb.torproject.org/user/brade/tor-browser.git/commitdiff/a9ccc52a47abcb55a1b39ca7aeb066dfcc861549

It has been tested on an old WinXP system where I reproduced the original problem.
Please review. If this fix looks good to others, we should pick it up for the next 4.0.x release as well as 4.5-alpha (when we can).

comment:23 Changed 5 years ago by mcs

Cc: mikeperry added
Status: assignedneeds_review

comment:24 Changed 5 years ago by mikeperry

Keywords: tbb-4.5-alpha added

comment:25 Changed 5 years ago by mcs

Keywords: MikePerry201411R added; MikePerry201410R removed

comment:26 Changed 5 years ago by mikeperry

Keywords: tbb-4.0-backport added
Resolution: fixed
Status: needs_reviewclosed

Ok, I merged this for 4.5-alpha-1 (since we need to rebundle it anyway for the circuit status UI).

comment:27 Changed 5 years ago by cypherpunks_backup

Nothing

Last edited 5 years ago by cypherpunks_backup (previous) (diff)

comment:28 Changed 5 years ago by mcs

I have some anecdotal evidence that the directory location specified by Mozilla's firefox.exe App Path (in the Registry) "wins out" over the PATH env variable. That could cause problems, especially once the updater depends on more DLLs (see #13379)

Note: See TracTickets for help on using tickets.