[tbb-bugs] #17931 [Tor Browser]: Tor Browser Hardened Crash

[Tor Bug Tracker & Wiki]

#17931: Tor Browser Hardened Crash
-------------------------------------------------+-------------------------
 Reporter:  pege                                 |          Owner:  tbb-
     Type:  defect                               |  team
 Priority:  Immediate                            |         Status:
Component:  Tor Browser                          |  needs_review
 Severity:  Blocker                              |      Milestone:
 Keywords:  tbb-hardened, tbb-crash,             |        Version:
  TorBrowserTeam201512R                          |     Resolution:
Parent ID:                                       |  Actual Points:
  Sponsor:                                       |         Points:
-------------------------------------------------+-------------------------

Comment (by arthuredelstein):

 Replying to [comment:8 mcs]:
 > Replying to [comment:6 arthuredelstein]:
 > > The bug here is exposed by an interaction between URL escaping and
 printf-like format specifiers. Here is what happens:
 > > ...
 >
 > Good work finding the root cause of the crash!

 To be precise, I am the root cause of the crash. Sorry about that.

 > I have not reviewed your patch yet, but you could reduce its size by
 continuing to use nsContentUtils::LogMessageToConsole() and just calling
 it like:
 >   nsContentUtils::LogMessageToConsole("%s", message.get());

 Great suggestion! Here's a patch that does that instead:
 https://github.com/arthuredelstein/tor-browser/commits/17931+1

 > But maybe that is too ugly and maybe we want to eliminate extra overhead
 (e.g., a call to PR_vsmprintf() that is not really needed).

 I think it's probably better to use this small patch. The extra overhead
 is pretty inconsequential, I think.

 > I also wonder if the call to nsContentUtils::LogMessageToConsole() in
 security/sandbox/chromium-shim/sandbox/win/loggingCallbacks.h at line 107
 is safe. But maybe Tor Browser does not use that code?

 I added a second patch to this branch, just in case.

 I'm suprised to see how little LogMessageToConsole is used in mozilla-
 central. Perhaps it would be safer to change it to a single-argument call
 that takes a plain string without format specifiers.

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