[tor-bugs] #4234 [Tor Browser]: Investigate the Firefox update process

[Tor Bug Tracker & Wiki]

#4234: Investigate the Firefox update process
-------------------------+-------------------------------------------------
     Reporter:           |      Owner:  mcs
  mikeperry              |     Status:  accepted
         Type:  task     |  Milestone:  TorBrowserBundle 2.3.x-stable
     Priority:  major    |    Version:
    Component:  Tor      |   Keywords:  tbb-bounty, tbb-usability,
  Browser                |  pantheon, chronos, tbb-firefox-
   Resolution:           |  patch,TorBrowserTeam201408,MikePerry201408R
Actual Points:           |  Parent ID:
       Points:           |
-------------------------+-------------------------------------------------

Comment (by mcs):

 Replying to [comment:36 mikeperry]:
 > Ok, I took a look at this, and overall it looks good. I have two
 questions though:
 >
 > In browser/installer/removed-files.in, it looks like you deleted
 msvcr100.dll. What is the effect of this and why was it done? Does it
 exclude that file from removal/update?

 Thanks for the review.  The effect is to avoid removing msvcr100.dll when
 an update is applied (same as for Firefox 24).  The reason we need a patch
 is because MOZ_MSVC_REDIST is defined differently in our builds compared
 to Firefox builds.


 > In toolkit/mozapps/update/updater/updater.cpp get_valid_path(), it looks
 like you allow symlink updates to specify paths in parent directories? Do
 we need to be worried about this? Can it be used by a rogue/broken MAR
 file to create symlinks outside of the TBB directory?

 That check is only skipped when the islinktarget parameter is true.  The
 only time true is passed is when parsing the symlink target (second call
 to get_valid_path() inside AddSymlink::Parse()).

 So we allow symlinks that point outside of the browser area to be created
 but the symlink itself cannot be located in a parent directory.  We needed
 to allow this because the meek symlinks contain .. in their targets.  Do
 you think we need a stronger check?  E.g., we could try to verify that the
 symlink target does not point to a directory that is outside the browser
 area.  Of course a rogue or broken MAR file can do all kinds of damage by
 replacing binaries with Bad Stuff.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/4234#comment:37>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online