Skip to content
Permalink
Browse files Browse the repository at this point in the history
XSS fixes:
-Patch render calls in CheckboxView, TableView and TableDataView to display escaped content; preserve <br> tags in table headers in TableView
-Escape the innerHTML call in string_measurement.js
-Escape element.html call when selecting read-only content in TableDataView
  • Loading branch information
Jesse Lankila committed Apr 22, 2015
1 parent 6b3df41 commit e6c49b5
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 11 deletions.
14 changes: 12 additions & 2 deletions utils/string_measurement.js
Expand Up @@ -19,9 +19,19 @@ Ember.mixin(Flame, {
return element;
},

measureString: function(string, parentClasses, elementClasses, additionalStyles) {
measureString: function(stringOrArray, parentClasses, elementClasses, additionalStyles) {
var escape = Handlebars.Utils.escapeExpression;
var measuredString;
// We also accept an array of strings and then return the width of the longest one by joining them with <br>.
if (Ember.isArray(stringOrArray)) {
measuredString = stringOrArray.reduce(function(currentStrings, nextString) {
return currentStrings + escape(nextString) + '<br>';
}, '');
} else {
measuredString = escape(stringOrArray);
}
var element = this._setupStringMeasurement(parentClasses, elementClasses, additionalStyles);
element.innerHTML = string;
element.innerHTML = measuredString;
return {
width: element.clientWidth,
height: element.clientHeight
Expand Down
2 changes: 1 addition & 1 deletion views/checkbox_view.js
Expand Up @@ -9,7 +9,7 @@ Flame.CheckboxView = Flame.ButtonView.extend({
buffer.push('<div class="flame-checkbox-box"></div>');
this.renderCheckMark(buffer);
buffer.push('<label class="flame-checkbox-label">');
buffer.push(Ember.isNone(this.get('title')) ? '' : this.get('title'));
buffer.push(Ember.isNone(this.get('title')) ? '' : Handlebars.Utils.escapeExpression(this.get('title')));
buffer.push('</label>');
},

Expand Down
5 changes: 1 addition & 4 deletions views/menu_view.js
Expand Up @@ -103,10 +103,7 @@ Flame.MenuView = Flame.Panel.extend(Flame.ActionSupport, {
return;
}
var itemTitleKey = this.get('itemTitleKey');
var allTitles = items.reduce(function(currentTitles, item) {
var nextTitle = Ember.get(item, itemTitleKey);
return currentTitles + nextTitle + '<br>';
}, '');
var allTitles = items.map(function(currentTitles, item) { Ember.get(item, itemTitleKey); });
// Give the menus a 16px breathing space to account for sub menu indicator, and to give some right margin (+18px for the padding)
return Flame.measureString(allTitles, 'ember-view flame-view flame-list-item-view flame-menu-item-view', 'title').width + 16 + 18;
},
Expand Down
8 changes: 6 additions & 2 deletions views/table_data_view.js
Expand Up @@ -456,7 +456,7 @@ Flame.TableDataView = Flame.View.extend(Flame.Statechart, {
var owner = this.get('owner');
var selection = owner.get('selection');
var dataCell = owner.get('selectedDataCell');
var readOnlyValue = owner.editableValue(dataCell, true);
var readOnlyValue = Handlebars.Utils.escapeExpression(owner.editableValue(dataCell, true));
this.selectionContent = selection.html();
selection.html(readOnlyValue);
selection.addClass('read-only is-selectable');
Expand Down Expand Up @@ -797,6 +797,7 @@ Flame.TableDataView = Flame.View.extend(Flame.Statechart, {
var defaultCellWidth = this.get('parentView.defaultColumnWidth');
var columnLeafs = this.get('parentView.content.columnLeafs');
var cellWidth;
var escape = Handlebars.Utils.escapeExpression;

var classes = 'flame-table';
if (!this.get('parentView.allowSelection')) classes += ' is-selectable';
Expand All @@ -818,10 +819,13 @@ Flame.TableDataView = Flame.View.extend(Flame.Statechart, {
while (div.firstChild) div.removeChild(div.firstChild);
div.appendChild(content);
content = div.innerHTML;
} else {
// Escape the cell content unless there is a deliberate implementation for cells with HTML content.
content = escape(content);
}
cssClassesString = cell.cssClassesString();
if (cell.inlineStyles) inlineStyles = cell.inlineStyles();
titleValue = (cell.titleValue && cell.titleValue() ? 'title="%@"'.fmt(cell.titleValue()) : '');
titleValue = (cell.titleValue && cell.titleValue() ? 'title="%@"'.fmt(escape(cell.titleValue())) : '');
} else {
content = '<span style="color: #999">...</span>';
}
Expand Down
5 changes: 3 additions & 2 deletions views/table_view.js
Expand Up @@ -319,7 +319,7 @@ Flame.TableView = Flame.View.extend(Flame.Statechart, {
}

if (this.get('content.title')) {
buffer.push('<div class="panel-title">%@</div>'.fmt(this.get('content.title')));
buffer.push('<div class="panel-title">%@</div>'.fmt(Handlebars.Utils.escapeExpression(this.get('content.title'))));
didRenderTitle = true;
}

Expand Down Expand Up @@ -438,7 +438,8 @@ Flame.TableView = Flame.View.extend(Flame.Statechart, {

headerLabel = header.get ? header.get('headerLabel') : header.label;
if (!headerLabel) headerLabel = "";

// We have to support <br> for row headers, so we'll replace them back after escaping
headerLabel = Ember.Handlebars.Utils.escapeExpression(headerLabel).replace(/&lt;br&gt;/g, '<br>');
buffer.attr('title', headerLabel.replace(/<br>/g, '\n'));

if (header.rowspan > 1) {
Expand Down

0 comments on commit e6c49b5

Please sign in to comment.