741874 - [Security Review][Action Item]WebSMS - gaia code review

Status: RESOLVED DUPLICATE of bug 754746

(mozilla.org :: Security Assurance, task)

Description

[Curtis Koenig [:curtisk-use curtis.koenig+bzATgmail.com]]]

Look at Gaia code - ensure dialer and sms can't be opened in an iframe maybe? Comes back to permissions model of B2G - gaia sms app author is vingtetun on IRC

Comment 1

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

Started gaia testing, note that some of the b2g features are not finished yet (for example window.open and window.prototype dont behave properly for mozbrowser).

Also a lot depends on the permission model which isnt finalised yet. This may delay this review, just FYI.

Comment 2

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

Initial review complete, code still under development though.

Only real issue so far is the approach to sanitizing untrusted HTML. Current approach uses an ad hoc function:

function escapeHTML(str, escapeQuotes) {
  var span = document.createElement('span');
  span.textContent = str;

  if (escapeQuotes)
    return span.innerHTML.replace(/"/g, '"').replace(/'/g, ''');
  return span.innerHTML;
}

Seems OK at first glance, but I can see this being error prone (for example, this will not help for any dynamically generated script content. Maybe this can be mitigated with content security policy.

Maybe just change adding a helper function might improve things, but its seems like something that could be leverage from elsewhere. e.g:

function escapeQuotedAttribute(str){
  return escapeHTML(str,true);
}

Comment 3

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

Another potential misuse case escapeHTML(str, true) will only work when escaping a single quoted attribute. If used for a list of attributes or similar this would be a risk.

It would be better to come up with a specific library used across gaia apps to escape untrusted content. Maybe something already exists that can be used for this purpose.

Comment 4

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

Closing this bug to merge it with the other gaia code review bugs