commit 8edeebec646e37161b78fabda6ab51f794df7fdb Author: Mike Perry <mikeperry-git@torproject.org> Date: Thu Oct 10 19:02:02 2019 -0500 Add Firefox 68 network audit notes --- audits/FF68_NETWORK_AUDIT | 311 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 311 insertions(+) diff --git a/audits/FF68_NETWORK_AUDIT b/audits/FF68_NETWORK_AUDIT new file mode 100644 index 0000000..f8965cb --- /dev/null +++ b/audits/FF68_NETWORK_AUDIT @@ -0,0 +1,311 @@ +Things to grep for in general when investigating changes: + +=============== Native DNS Portion ============= + +PR_GetHostByName +PR_GetIPNodeByName +PR_GetAddrInfoByName +PR_StringToNetAddr (itself is good as it passes AI_NUMERICHOST to getaddrinfo. No resolution.) + +MDNS +TRR (DNS Trusted Recursive Resolver) + +Direct Paths to DNS resolution: +nsDNSService::Resolve +nsDNSService::AsyncResolve +nsHostResolver::ResolveHost + +============ Misc Socket Portion ============== + +SOCK_ +SOCKET_ +_SOCKET +UDPSocket +TCPSocket + PR_NewTCPSocket + AsyncTCPSocket + +Misc PR_Socket + +=========== Misc XPCOM Portion ================ + +Misc XPCOM (including commands for pre-diff review approach) + *SocketProvider + grep -R udp-socket . + grep -R tcp-socket . + grep for tcpsocket + grep -R "NS_" | grep SOCKET | grep "_C" + grep -R "@mozilla.org/network/" . | grep socket | grep -v udp-socket + +============ Rust Portion ================ + +Rust + - XXX: What do we grep for here? Or do we rely on Ritter's compile-time tool? + - Check for new sendmsg and recvmsg usage + +============ Android Portion ============= + +Android Java calls + - URLConnection + - XXX: getInputStream? other methods? + - HttpURLConnection + - UrlConnectionDownloader + - ch.boye.httpclientandroidlib.impl.client.* (look for execute() calls) + - grep -n openConnection\( mobile/android/thirdparty/ + - java.net + - java.net.URL -- has SEVERAL proxy bypass URL fetching methods :/ + - javax.net + - ch.boye.httpclientandroidlib.conn.* (esp ssl) + - ch.boye.httpclientandroidlib.impl.conn.* (esp ssl) + - Sudden appearance of thirdparty libs: + - OkHttp + - Retrofit + - Glide + - com.amitshekhar.android + - IntentHelper + - openUriExternal (can come from GeckoAppShell too) + - getHandlersForMimeType + - getHandlersForURL + - getHandlersForIntent + - android.content.Intent - too common; instead find launch methods: + - startActivity + - startActivities + - sendBroadcast + - sendOrderedBroadcast + - startService + - bindService + - android.app.PendingIntent + - android.app.DownloadManager + - ActivityHandlerHelper.startIntentAndCatch + + + +============ Regression/Prior Vuln Review ========= + +Review proxy bypass bugs; check for new vectors to look for: + - https://trac.torproject.org/projects/tor/query?keywords=~tbb-proxy + +------------------------------------------------------------------------------- +I followed the diff approach by doing `git diff esr60 esr68` and then +going over all the changes containing the above mentioned potentially dangerous +calls and features. + +Direct Paths to DNS resolution: + + PR_GetHostByName + + No new cals in diff +PR_GetIPNodeByName + + No new calls in diff +PR_GetAddrInfoByName + + Edited calls in diff, but no change +PR_StringToNetAddr (itself is good as it passes AI_NUMERICHOST to getaddrinfo. No resolution.) + + One change in NSS unit tests ("127.0.0.1" to higher level call) + +MDNS: + + No substantive changes + +nsDNSService: + - nsDNSServiceDiscovery appears to support fallback JS mDNS option + + nsDNSServiceDiscovery itself is not new.. + + nsDNSService has an mProxyType now + - nsDNSService::AsyncResolveInternal()? + + Patch blocks this (XXX: Maybe should move to PreprocessHostName) + + nsDNSAsyncRequest + - nHostResolver::ResolveHost + + not used outside dnsservice + + nsDNSService::ResolveInternal() + + ResolveHost() with type now... + + Patched + + AsyncResolve, AsyncResolveNative, AsyncResolveByType + + AsyncResolveInternal() + + Patched + + Resolve() + + ResolveNative + + ResolveInternal() + + Patched + + What about our dns service patch? where did that go? + + https://gitweb.torproject.org/tor-browser.git/commit/?h=tor-browser-68.1.0es... + + https://gitweb.torproject.org/tor-browser.git/commit/?h=tor-browser-60.8.0es... +nsHostResolver: + + Only used by DNS service + + +SOCK_ + + libvpx has unit tests that use sockets + + many changes due to moved webrtc code.. + + All disabled and not compiled in + + Lots of python unittests + + Sctp code has sockets.. Is this used? + + by webrtc, which we disable + - rust/cloudabi has some... + + recordreplay has some but they are AF_UNIX + +SOCKET_ + + Again, lots due to moved WebRTC code + + Some android sockets in "LeanPlum" android lib + + leanplum is disabled + + Curl is in the tree now? lol + + Some sandboxing constants + +_SOCKET + - media/audioipc in rust? + - mio in rust? + + is media/mtransport/nr_socket_prsock.cpp off with webrtc? + + looks like yes + - IOActivityMonitor? + + Looks benign and also not new + +UDPSocket + - XXX: devtools debugger discovery still uses it + - ./devtools/shared/discovery/discovery.js + + webrtc moved code + + dom.udpsocket.enabled still false + - fallback multicastDNS uses it (not new) +TCPSocket + + devtools adbsocket uses, but to localhost + + mtransport uses, but disabled for webrtc + + Tests use + +SocketTransport + + IOactivityMonitor, NetworkActivityMonitor? (old) + - XXX: presentation control service? I think this is old but moved + - dom/presentation/PresentationTCPSessionTransport.cpp + - App to app service.. + - Dashboard + - netwerk/base/Dashboard.cpp + + Only examine opened sockets and cached DNS + - XXX: Roku control (old) + - ./toolkit/modules/secondscreen/RokuApp.jsm + - NetworkConnectivityService?? This looks new + - ./netwerk/base/NetworkConnectivityService.cpp + + Uses DNS service and nsIChannels.. Proxy safe + - TCPFastOpen (old, but flagged before) + - ./netwerk/base/TCPFastOpenLayer.cpp + - AttachTCPFastOpenIOLayer + + Appears to attach to nsSocketTransport such that it would only fast + open to the socks port.. + - TCPFastOpenConnect + + Only used via nsSocketTransport + +Extra: + PR_Socket() + + None + PR_Connect()? + + Minor changes + + +Misc XPCOM (including commands for pre-diff review approach) + + *SocketProvider + + Only minor changes + grep -R udp-socket . + + minor changes + grep -R tcp-socket . + + docstring only + grep for tcpsocket + + adb uses for 127.0.0.1 connection + + webrtc + grep -R "NS_" | grep SOCKET | grep "_C" + + indent and thread check changes; removed code + grep -R "@mozilla.org/network/" . | grep socket | grep -v udp-socket + + minor changes to spdy and unittests + +Rust + + Check for new sendmsg and recvmsg usage + - XXX: only in mio, audioipc.. + - XXX: ask tjr to re-run tool? + +Android Java calls + - URLConnection (and HttpURLConnection) + + updater code moved; still looks ok + + BitmapConnection extends in mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoAppShell.java + + Some DRM thing + + Crashreporter + + leanplum (disabled) + + DefaultHttpDataSource (used only by exoplayer/HLS, which is disabled) + + tests + + grep -n openConnection\( mobile/android/thirdparty/ + + leanplum (disabled) + + UrlConnectionDownloader in mobile/android/thirdparty/com/squareup/picasso/ + + ch.boye.httpclientandroidlib.impl.client.* (look for execute() calls) + + BaseResource.java (is this proxied? Who calls this?) + + Lots of consumers.. + - XXX: Patch going back in: https://bugs.torproject.org/31934 + + leanplum (disabled) + + Adjust SDK uses execute directly + + https://bugs.torproject.org/25906 disabled + + java.net + + mobile/android/geckoview/src/thirdparty/java/com/google/android/exoplayer2/upstream/UdpDataSource.java + + Not used anywhere + + also: https://gitweb.torproject.org/tor-browser.git/commit/mobile/android/geckovie... + + also https://gitweb.torproject.org/tor-browser.git/commit/mobile/android/moz.conf... + - java.net.URL + - XXX: GeckoApplication.downloadImageForSetImage uses URL.openStream() + - XXX: GeckoActionProvider.downloadImageForIntent uses java.net.URL.openStream() + - XXX: GeckAppShell has many wrappers to create inputstreams from URLConnections + - Do these always have to be opened/connected first? + - XXX: GeckoMediaDrmBridgeV21.java - uses android.media.MediaDrm which seems to fetch stuff?? + - XXX: BitmapUtils.decodeUrl uses openStream for non-jar urls + - XXX: GeckoJarReader - tons of use.. Can this be used on remote jars? + - XXX: AbstractCommunicator.openConnectionAndSetHeaders() - uses url.openConnection() + - XXX: AbstractCommunicator.sendData() - uses url.getOutputStream().. maybe ok? + + javax.net + + TLSSocketFactory from sync + + not used + + ch.boye.httpclientandroidlib.conn.* (esp ssl) + + Not used + + ch.boye.httpclientandroidlib.impl.conn.* (esp ssl) + + Not used + - IntentHelper + + openUriExternal prompts if private browsing mode enabled + - XXX: Do we enable this? Can users turn it off? + - XXX: ./mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/menu/ActivityStreamContextMenu.java + - Uses intents to direct fetch URL + - XXX: Sets "show in private browsing mode" prompt to false + - XXX: ./mobile/android/base/java/org/mozilla/gecko/BrowserApp.java + - Also does telemetry via IntentHelper.openURIExternal + - XXX: Delegates to BrowserAppDelegate list in onNewIntent()... What's in this + list? + - XXX: ./mobile/android/base/java/org/mozilla/gecko/ChromeCastDisplay.java + - XXX: Looks like it might launch external service? + - XXX: ./mobile/android/base/java/org/mozilla/gecko/home/HomeFragment.java + - getHandlersFor* + - HelperApps.jsm + + browser.js uses prompt + + HelperAppDialog.js + - android.content.Intent? OMG everywhere :( + - startActivity + - XXX: ActivityHandlerHelper - Good candidate to patch for external + activities + - XXX: BrowserApp.onUrlOpenWithRefferer - Might be able to launch other + apps if OPEN_WITH_INTENT flag is set? + - XXX: CustomTabsActivity.java - Several methods + - XXX: WebAppActivity.onLoadRequest + - XXX: BasicGeckoViewPrompt.onFilePrompt() + - XXX: GeckoViewActivity.onExternalResponse() + + startActivities + + not used + + sendBroadcast + + Some wifi scanning/GPS stuff but is local broadcast.. + + sendOrderedBroadcast + + not used + + startService + + all use seems targeted to sub-components + - bindService + - XXX: SurfaceAllocator - no idea what is happening here :/ + - XXX: RemoteManager - no idea what is happening here :/ +- android.app.PendingIntent + - XXX: ChromeCastDisplay.java - probably want to make sure this is disabled? + - It uses RemotePresentationService + - XXX: CustomTabsActivity.performPendingIntent - again, hard to tell what is + happening here :/ +- android.app.DownloadManager + - XXX: BrowserApp.java uses it + - XXX: DownloadsIntegration.java uses it, but has a check for useSystemDownloadManager() ++ ActivityHandlerHelper.startIntentAndCatch + + Used only by other wrappers already covered + + + +TRR: + - assuming disabled by pref; deferring audit + +