1172091 - Audit our content scripts/message handlers

Status: RESOLVED FIXED

(Firefox for iOS :: General, defect)

Description

[Brian Nicholson (:bnicholson)]

Before releasing v1, we need to go over all of our user scripts and carefully verify that we aren't exposing anything dangerous to web pages. In particular, the in-content webkit object used for sending messages back to the app process is exposed to web pages, so we also need to look through the receiving end (BrowserHelpers) to make sure we robustly handle any message that may be sent back to us. Any message posted via webkit.messageHandlers could be malicious, so we need to filter them accordingly.

Comment 1

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

This includes looking at the Swift handlers for those messages to make sure they handle bad input data correctly.

This includes things like:

* Receiving nil/null and assuming that is not possible
* Receiving different data types than expected
* Receiving longer or shorter data then expected
* Receiving correct data structures but with invalid data

This will be a fun thing for Whistler I think. We can also ask some of the security team folks to help with this, since they have been looking at issues like this across our products.

Comment 2

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

This is what we inject:

* ContextMenu.js & ContextMenuHelper.swift
* Favicons.js & FaviconManager.swift
* FxASignIn.js & FxAContentViewController.js
* LoginsHelper.js & LoginsHelper.swift
* Readability.js & ReadabilityBrowserHelper.js & ReadabilityBrowserHelper.swift
* Readability.js & ReaderMode.js & ReaderMode.swift
* WindowCloseHelper.js & WindowCloseHelper.swift

Comment 3

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

WindowCloseHelper.js & WindowCloseHelper.swift

These two look pretty sane. Code is executed in an anonymous function. No data is passed between webview and native. The only message it sends is 'null' as a notification.

Comment 4

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

FxASignIn.js & FxAContentViewController.swift

The main problem with FxAContentViewController.swift is that it does zero validation of the data it receives in userContentController(didReceiveScriptMessage).

I'm not too worried about someone hijacking the fxa-content-server. But if it for some reason sends back an unexpected JSON response, we will likely crash because we assume specific keys are in returned dictionaries and assume they are of specific types.

Highly recommend to write a strict parser from FxA Content Server responses and turn the JSON into Swift structs to encapsulate the responses.

Comment 5

[Brian Nicholson (:bnicholson)]

Sounds like we'll be dumping some findings in here. Restricting access so we don't give people a laundry list of 0-day exploits for release :)

Comment 6

[Richard Newman [:rnewman]]

This'll do for v1.