Re: [tor-bugs] #24398 [Applications/Tor Browser]: Plugin-container exhausts memory leading to thrash and/or crash on Linux

[Tor Bug Tracker & Wiki]

#24398: Plugin-container exhausts memory leading to thrash and/or crash on Linux
--+--
 Reporter:  gk|  Owner:  tbb-team
 Type:  defect| Status:  closed
 Priority:  Medium|  Milestone:
Component:  Applications/Tor Browser  |Version:
 Severity:  Normal| Resolution:  fixed
 Keywords:  tbb-crash, TorBrowserTeam201712R  |  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+--
Changes (by gk):

 * status:  needs_review => closed
 * resolution:   => fixed


Comment:

 Applied to `tor-browser-52.5.2esr-7.5-2` (as commit
 cef2fe28a238d7e445d7c5b4292bfe27c1b71bca). Thanks!

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #24398 [Applications/Tor Browser]: Plugin-container exhausts memory leading to thrash and/or crash on Linux

[Tor Bug Tracker & Wiki]

#24398: Plugin-container exhausts memory leading to thrash and/or crash on Linux
--+
 Reporter:  gk|  Owner:  tbb-team
 Type:  defect| Status:
  |  needs_review
 Priority:  Medium|  Milestone:
Component:  Applications/Tor Browser  |Version:
 Severity:  Normal| Resolution:
 Keywords:  tbb-crash, TorBrowserTeam201712R  |  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+

Comment (by mcs):

 r=brade, r=mcs
 We confirmed that the patch avoids the recursion problem. Because the
 devtools code cannot load file URLs such as the userContent.css
 stylesheet, things still do not work correctly... but at least we avoid
 infinite recursion.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #24398 [Applications/Tor Browser]: Plugin-container exhausts memory leading to thrash and/or crash on Linux

[Tor Bug Tracker & Wiki]

#24398: Plugin-container exhausts memory leading to thrash and/or crash on Linux
--+
 Reporter:  gk|  Owner:  tbb-team
 Type:  defect| Status:
  |  needs_review
 Priority:  Medium|  Milestone:
Component:  Applications/Tor Browser  |Version:
 Severity:  Normal| Resolution:
 Keywords:  tbb-crash, TorBrowserTeam201712R  |  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+

Comment (by mcs):

 Kathy and I do not always see the memory growth, but when running with
 `--verbose`` we see many lines like the following:
  newChannelForURL@resource://gre/modules/commonjs/toolkit/loader.js ->
 resource://devtools/shared/DevToolsUtils.js:567:12
 and then a few like this:
  console.error: Protocol error (unknownError): too much recursion

 So I think we are reproducing the bug. We did not do anything special; if
 you load a page you should have a plugin-container process (assuming
 multiprocess mode is working).

 We are going to rebuild with the proposed fix in place and see if the
 problem goes away.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #24398 [Applications/Tor Browser]: Plugin-container exhausts memory leading to thrash and/or crash on Linux

[Tor Bug Tracker & Wiki]

#24398: Plugin-container exhausts memory leading to thrash and/or crash on Linux
--+
 Reporter:  gk|  Owner:  tbb-team
 Type:  defect| Status:
  |  needs_review
 Priority:  Medium|  Milestone:
Component:  Applications/Tor Browser  |Version:
 Severity:  Normal| Resolution:
 Keywords:  tbb-crash, TorBrowserTeam201711R  |  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+
Changes (by gk):

 * status:  new => needs_review


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #24398 [Applications/Tor Browser]: Plugin-container exhausts memory leading to thrash and/or crash on Linux

[Tor Bug Tracker & Wiki]

#24398: Plugin-container exhausts memory leading to thrash and/or crash on Linux
--+--
 Reporter:  gk|  Owner:  tbb-team
 Type:  defect| Status:  new
 Priority:  Medium|  Milestone:
Component:  Applications/Tor Browser  |Version:
 Severity:  Normal| Resolution:
 Keywords:  tbb-crash, TorBrowserTeam201711R  |  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+--

Comment (by gk):

 I gave the STR a quick test and for some reason I did not have a plugin-
 container process to begin with. So, we might need slightly adapted STR
 for testing the patch.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #24398 [Applications/Tor Browser]: Plugin-container exhausts memory leading to thrash and/or crash on Linux

[Tor Bug Tracker & Wiki]

#24398: Plugin-container exhausts memory leading to thrash and/or crash on Linux
--+--
 Reporter:  gk|  Owner:  tbb-team
 Type:  defect| Status:  new
 Priority:  Medium|  Milestone:
Component:  Applications/Tor Browser  |Version:
 Severity:  Normal| Resolution:
 Keywords:  tbb-crash, TorBrowserTeam201711R  |  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+--
Changes (by gk):

 * Attachment "tor-browser-runaway-memory-allocation.patch" added.


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

[tor-bugs] #24398 [Applications/Tor Browser]: Plugin-container exhausts memory leading to thrash and/or crash on Linux

[Tor Bug Tracker & Wiki]

#24398: Plugin-container exhausts memory leading to thrash and/or crash on Linux
-+-
 Reporter:  gk   |  Owner:  tbb-team
 Type:  defect   | Status:  new
 Priority:  Medium   |  Milestone:
Component:  Applications/Tor |Version:
  Browser|   Keywords:  tbb-crash,
 Severity:  Normal   |  TorBrowserTeam201711R
Actual Points:   |  Parent ID:
   Points:   |   Reviewer:
  Sponsor:   |
-+-
 We got a nice bugreport from a cypherpunk on our tbb-dev mailing list
 (https://lists.torproject.org/pipermail/tbb-dev/2017-November/000673.html)
 with a patch up for review (which I'll attach in a minute) (the problem
 seems to be caused by our workaround for #24052):
 {{{
 STR
 ---

 1. Run one of the platforms affected by the recent "tormoil" vulnerability
 (I
tested this on a GNU/Linux distro).

 2. Under Tor Browser 7.0.10's installation directory, create
 `Browser/TorBrowser/Data/Browser/profile.default/chrome/userContent.css`
with the following content (you can use whatever you want here, just
 adjust
the next steps accordingly):

  body {
background-color: blue !important;
color: white !important;
  }

 3. Start Tor Browser.

 4. Confirm that the content background looks blue.

 5. Right click the blue background on an empty spot (body element) and
 choose
"Inspect Element".

 6. Wait until the Inspector window shows up.  Observe memory consumption
 of
process "plugin-container" continually rise.

 Analysis
 

 I think this regression is caused by the workaround [0] for the recent
 critical
 vulnerability [1], but it has exposed what looks to me (not being privy to
 the
 details of "tormoil") like another bug, this one in javascript code.

   0: https://gitweb.torproject.org/tor-browser.git/commit/?h=tor-
 browser-52.5.0esr-7.0-1=643117230bb3402c997f065980db1eec19c7a6ed
   1: https://trac.torproject.org/projects/tor/ticket/24052

 `newChannelForURL` in DevToolsUtils.js (packed inside
 `Browser/browser/omni.ja`) will recursively call itself when
 `NetUtil.newChannel` raises an exception, prepending "file://" to the
 input
 URL:

   /**
* Opens a channel for given URL. Tries a bit harder than
 NetUtil.newChannel.
*
* @param {String} url - The URL to open a channel for.
* @param {Object} options - The options object passed to @method fetch.
* @return {nsIChannel} - The newly created channel. Throws on failure.
*/
   function newChannelForURL(url, { policy, window, principal }) {
 var securityFlags =
 Ci.nsILoadInfo.SEC_ALLOW_CROSS_ORIGIN_DATA_IS_NULL;

 let uri;
 try {
   uri = Services.io.newURI(url, null, null);
 } catch (e) {
   // In the xpcshell tests, the script url is the absolute path of the
 test
   // file, which will make a malformed URI error be thrown. Add the
 file
   // scheme to see if it helps.
   uri = Services.io.newURI("file://" + url, null, null);
 }

 [...]

 try {
   return NetUtil.newChannel(channelOptions);
 } catch (e) {
   // In xpcshell tests on Windows,
 nsExternalProtocolHandler::NewChannel()
   // can throw NS_ERROR_UNKNOWN_PROTOCOL if the external protocol
 isn't
   // supported by Windows, so we also need to handle the exception
 here if
   // parsing the URL above doesn't throw.
   return newChannelForURL("file://" + url, { policy, window, principal
 });
 }
   }

 With the mentioned patch, the call to `NetUtil.newChannel` raises an
 exception.  This results in infinite recursion coupled with rapid (though
 linear) memory consumption.  Thus, `plugin-container` will, in just a few
 seconds, exhaust all available memory.

 Partial fix
 ---

 AFAICT, the current patch for "tormoil" is just a hurried stop-gap
 workaround
 and as such it is acceptable, and somewhat expected, for it to cause
 other,
 less severe, kinds of breakage.  However, ISTM that the code in
 `newChannelForURL` is buggy regardless: the recursion has no (evident)
 termination condition.

 The comment before the recursive call says that it is needed due to some
 tests
 for which `newChannel` can raise `NS_ERROR_UNKNOWN_PROTOCOL`.  So maybe it
 should only catch the exception (and make the recursive call) if the error
 is
 that one and `url` does not already start with "file://".  The attached
 patch
 does this.  It does not fix the regression (the Developer Toolbox still
 fails
 to show the appropriate styles and stylesheets), but at least fixes the
 DoS
 caused by the runaway memory allocation.
 }}}

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor