[tor-bugs] #31144 [Applications/Tor Browser]: ESR68 Network Code Review

[Tor Bug Tracker & Wiki]

#31144: ESR68 Network Code Review
-------------------------------------------------+-------------------------
 Reporter:  pili                                 |          Owner:  tbb-
                                                 |  team
     Type:  task                                 |         Status:
                                                 |  needs_review
 Priority:  Very High                            |      Milestone:
Component:  Applications/Tor Browser             |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  TorBrowserTeam201910R, tbb-9.0       |  Actual Points:
  -alpha-must                                    |
Parent ID:                                       |         Points:  10
 Reviewer:                                       |        Sponsor:
-------------------------------------------------+-------------------------

Comment (by mikeperry):

 Replying to [comment:15 sysrqb]:
 > (continuing...)
 >
 > Replying to [comment:11 mikeperry]:

 > >    - BrowserApp.java (see also onNewIntent() delegation to
 BrowserAppDelegates list)
 >
 > Can you provide a link for this? I'm missing it somehow.

 In ./mobile/android/base/java/org/mozilla/gecko/BrowserApp.java search for
 BrowserAppDelegate (sorry, no s). That type is used to create a list of
 things that have Activity related things passed to them. I think it might
 be harmless though.

 > > 4. android.content.Intent startActivity() usage (may or may not be
 unsafe depending on circumstance :/)
 > >    - ActivityHandlerHelper - Good candidate to patch for external
 activities, but not everything uses it :/
 > >    - BrowserApp.onUrlOpenWithRefferer () - Might be able to launch
 other apps if OPEN_WITH_INTENT flag is set?
 >
 > Caught by forcing above prompt.

 Wait, both of these call startActivity() directly with an intent. Forcing
 the prompy from IntentHelper will NOT catch these.

 If ActivityHandlerHelper was patched to call into IntentHelper (or add its
 own prompt), then all the things that us it would prompt, but BrowserApp
 doesn't use either of the Helper classes to handle its Intents.

 > > 6. android.app.PendingIntent
 > >    - ChromeCastDisplay.java - probably want to make sure this is
 disabled?
 >
 > Disabled.
 >
 > >    - CustomTabsActivity.performPendingIntent - again, hard to tell
 what is happening here
 >
 > These seem like they could be arbitrary actions.

 Hrmm.. should we patch that somehow, or assume it is handled when the
 Intent is finally delivered?

 > > 7. android.app.DownloadManager
 > >    - DownloadsIntegration.java uses it, but has a check for
 useSystemDownloadManager() to avoid using it
 > >    - BrowserApp.java uses it to download items without any checks
 > >
 >
 > This is controlled by
 `browser.download.forward_oma_android_download_manager` which is false.
 (https://bugzilla.mozilla.org/show_bug.cgi?id=1253684 which is
 restricted?). I'll add this into the override file, just so we aren't
 surprised by a change later.

 Are we sure that pref governs both usages of the download mamager? I did
 not see any checks in the BrowserApp itself.

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