[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:  TorBrowserTeam201910, tbb-9.0        |  Actual Points:
  -alpha-must                                    |
Parent ID:                                       |         Points:  10
 Reviewer:                                       |        Sponsor:
-------------------------------------------------+-------------------------
Changes (by mikeperry):

 * status:  new => needs_review


Comment:

 Ok I believe I have completed the android portion now. Here is the full
 list of found items, removing the ones GeKo said were fixed:

 1. Rust lib check
 2. java.net.URL stream usage (which looks like it bypasses the proxy)
    - GeckoApplication.downloadImageForSetImage uses URL.openStream()
    - GeckoActionProvider.downloadImageForIntent uses
 java.net.URL.openStream()
    - GeckAppShell has many wrappers to create inputstreams from
 URLConnections (but these may need to be opened first?)
    - GeckoMediaDrmBridgeV21.java - uses android.media.MediaDrm which seems
 to fetch stuff??
    - BitmapUtils.decodeUrl uses openStream for non-jar urls
    - GeckoJarReader - tons of stream use.. Can this be used on remote
 jars?
    - AbstractCommunicator.openConnectionAndSetHeaders() - uses
 url.openConnection() (I think we patched this one in #31934?)
    - AbstractCommunicator.sendData() - uses url.getOutputStream().. maybe
 ok?
 3. IntentHelper openUriExternal usage - maybe we should just patch this to
 always prompt?
    - ActivityStreamContextMenu.java
    - BrowserApp.java (see also onNewIntent() delegation to
 BrowserAppDelegates list)
    - ChromeCastDisplay.java
    - HomeFragment.java
 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?
    - CustomTabsActivity.java - Several methods emit potentially external
 Intents
    - WebAppActivity.onLoadRequest()
    - BasicGeckoViewPrompt.onFilePrompt()
    - GeckoViewActivity.onExternalResponse()
 5. Intent bindService() usage:
    - SurfaceAllocator - no idea what is happening here :/
    - RemoteManager - no idea what is happening here :/
 6. android.app.PendingIntent
    - ChromeCastDisplay.java - probably want to make sure this is disabled?
    - CustomTabsActivity.performPendingIntent - again, hard to tell what is
 happening here
 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

 I committed a rubric of what I did for future audits/tooling here:
 https://gitweb.torproject.org/tor-browser-
 spec.git/tree/audits/NETWORK_AUDIT_RUBRIC

 I also committed my notes here: https://gitweb.torproject.org/tor-browser-
 spec.git/tree/audits/FF68_NETWORK_AUDIT

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