810652 - XSS in Settings App via a web app manifest fields

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Settings, defect, P2)

Description

[Paul Theriault [:pauljt] (no longer reading bugmail)]

The settings app uses data in the app object (populated from navigator.mozApps.mgmt.getAll()) without sanitising the data. Injecting HTML in the application name, and possibly other manifest fields results in script injection in the context of the settings app. Including the following line in a web app's manifest results in an alert dialog when the user navigates to Settings -> Application Permissions. 

"name": "<img src=a onerror=alert(1)>"

I think the offending code is around here: https://github.com/mozilla-b2g/gaia/blob/master/apps/settings/js/apps.js#L79

But there is lots of usage of data from the manifest in that file, which all needs to be escaped. Also urls (now set by the code from bug 809293) need to be sanitized to make sure they are not something like javascript: or data: URIs.

CSP will, in theory, prevent these attacks once we have a default policy of no inline script enforced. But CSP should be a secondary defense, not the only protection.

Comment 1

[Jason Smith [:jsmith]]

Hmm...this is the third XSS bug I've seen come up. I feel like we need to do full-blown analysis across apps for this. Do we have any static analysis checkers we could use to catch a lot of possible XSS bugs up front to get started?

Comment 2

[Paul Theriault [:pauljt] (no longer reading bugmail)]

(In reply to Jason Smith [:jsmith] from comment #1)
> Hmm...this is the third XSS bug I've seen come up. I feel like we need to do
> full-blown analysis across apps for this. Do we have any static analysis
> checkers we could use to catch a lot of possible XSS bugs up front to get
> started?

A couple of us from the security assurance team are currently reviewing source code across the all of the gaia apps with the main goal of trying to identify XSS bugs. We do not currently have any automated tools as far as I am aware. I've not had a lot of success with automated tools in detecting DOM-based XSS in the past but if anyone has any recommendations I'm open to suggestions.

Comment 3

[Paul Theriault [:pauljt] (no longer reading bugmail)]

Also, there are probably some more general guidelines that we can come with to reduce the risk of this issue occurring, and making it easier to review for (e.g. less use of innerHTML and more use of createElement()&setAttribute(), using common libraries for escaping strings etc).

Comment 4

[Etienne Segonzac (:etienne)]

Attachment: Patch proposal

[Approval Request Comment]
User impact if declined: Security risk
Testing completed: App permissions management tested on device

Comment 5

[Etienne Segonzac (:etienne)]

Comment on attachment 680996 [details] [diff] [review]
Patch proposal

Canceling the approval request since this is know basecamp-blocker-+

Comment 6

[Etienne Segonzac (:etienne)]

https://github.com/mozilla-b2g/gaia/pull/6408

Comment 7

[Paul Theriault [:pauljt] (no longer reading bugmail)]

Unhiding this.