SECURITY: Backported XSS fixes from Handlebars

This commit is contained in:
Robin Ward 2015-11-24 16:07:47 -05:00
parent e9f80464b4
commit 189595466c
22 changed files with 71 additions and 30 deletions

View File

@ -41,7 +41,6 @@ gem 'onebox'
gem 'ember-rails' gem 'ember-rails'
gem 'ember-source', '1.12.1' gem 'ember-source', '1.12.1'
gem 'handlebars-source', '2.0.0'
gem 'barber' gem 'barber'
gem 'babel-transpiler' gem 'babel-transpiler'

View File

@ -119,7 +119,6 @@ GEM
given_core (3.5.4) given_core (3.5.4)
sorcerer (>= 0.3.7) sorcerer (>= 0.3.7)
guess_html_encoding (0.0.11) guess_html_encoding (0.0.11)
handlebars-source (2.0.0)
hashie (3.4.0) hashie (3.4.0)
highline (1.7.1) highline (1.7.1)
hike (1.2.3) hike (1.2.3)
@ -418,7 +417,6 @@ DEPENDENCIES
flamegraph flamegraph
foreman foreman
gctools gctools
handlebars-source (= 2.0.0)
highline highline
hiredis hiredis
htmlentities htmlentities

View File

@ -16,7 +16,7 @@ export default Ember.Component.extend({
render(buffer) { render(buffer) {
buffer.push("<div class='ace'>"); buffer.push("<div class='ace'>");
if (this.get('content')) { if (this.get('content')) {
buffer.push(Handlebars.Utils.escapeExpression(this.get('content'))); buffer.push(Discourse.Utilities.escapeExpression(this.get('content')));
} }
buffer.push("</div>"); buffer.push("</div>");
}, },

View File

@ -22,7 +22,7 @@ export default Ember.Controller.extend({
returned = "<pre class='badge-query-plan'>"; returned = "<pre class='badge-query-plan'>";
_.each(raw, function(linehash) { _.each(raw, function(linehash) {
returned += Handlebars.Utils.escapeExpression(linehash["QUERY PLAN"]); returned += Discourse.Utilities.escapeExpression(linehash["QUERY PLAN"]);
returned += "<br>"; returned += "<br>";
}); });
@ -32,7 +32,7 @@ export default Ember.Controller.extend({
processed_sample: Ember.computed.map('model.sample', function(grant) { processed_sample: Ember.computed.map('model.sample', function(grant) {
var i18nKey = 'admin.badges.preview.grant.with', var i18nKey = 'admin.badges.preview.grant.with',
i18nParams = { username: Handlebars.Utils.escapeExpression(grant.username) }; i18nParams = { username: Discourse.Utilities.escapeExpression(grant.username) };
if (grant.post_id) { if (grant.post_id) {
i18nKey += "_post"; i18nKey += "_post";
@ -41,7 +41,7 @@ export default Ember.Controller.extend({
if (grant.granted_at) { if (grant.granted_at) {
i18nKey += "_time"; i18nKey += "_time";
i18nParams.time = Handlebars.Utils.escapeExpression(moment(grant.granted_at).format(I18n.t('dates.long_with_year'))); i18nParams.time = Discourse.Utilities.escapeExpression(moment(grant.granted_at).format(I18n.t('dates.long_with_year')));
} }
return I18n.t(i18nKey, i18nParams); return I18n.t(i18nKey, i18nParams);

View File

@ -16,14 +16,14 @@ Discourse.StaffActionLog = Discourse.Model.extend({
formatted += this.format('admin.logs.staff_actions.previous_value', 'previous_value'); formatted += this.format('admin.logs.staff_actions.previous_value', 'previous_value');
} }
if (!this.get('useModalForDetails')) { if (!this.get('useModalForDetails')) {
if (this.get('details')) formatted += Handlebars.Utils.escapeExpression(this.get('details')) + '<br/>'; if (this.get('details')) formatted += Discourse.Utilities.escapeExpression(this.get('details')) + '<br/>';
} }
return formatted; return formatted;
}.property('ip_address', 'email', 'topic_id', 'post_id'), }.property('ip_address', 'email', 'topic_id', 'post_id'),
format: function(label, propertyName) { format: function(label, propertyName) {
if (this.get(propertyName)) { if (this.get(propertyName)) {
return ('<b>' + I18n.t(label) + ':</b> ' + Handlebars.Utils.escapeExpression(this.get(propertyName)) + '<br/>'); return ('<b>' + I18n.t(label) + ':</b> ' + Discourse.Utilities.escapeExpression(this.get(propertyName)) + '<br/>');
} else { } else {
return ''; return '';
} }

View File

@ -19,7 +19,7 @@ export default Ember.View.extend({
let formattedLogs = this.get("formattedLogs"); let formattedLogs = this.get("formattedLogs");
for (let i = this.get("index"), length = logs.length; i < length; i++) { for (let i = this.get("index"), length = logs.length; i < length; i++) {
const date = logs[i].get("timestamp"), const date = logs[i].get("timestamp"),
message = Handlebars.Utils.escapeExpression(logs[i].get("message")); message = Discourse.Utilities.escapeExpression(logs[i].get("message"));
formattedLogs += "[" + date + "] " + message + "\n"; formattedLogs += "[" + date + "] " + message + "\n";
} }
// update the formatted logs & cache index // update the formatted logs & cache index

View File

@ -38,10 +38,10 @@ export default Ember.Component.extend({
description: function() { description: function() {
const badgeName = this.get("notification.data.badge_name"); const badgeName = this.get("notification.data.badge_name");
if (badgeName) { return Handlebars.Utils.escapeExpression(badgeName); } if (badgeName) { return Discourse.Utilities.escapeExpression(badgeName); }
const title = this.get('notification.data.topic_title'); const title = this.get('notification.data.topic_title');
return Ember.isEmpty(title) ? "" : Handlebars.Utils.escapeExpression(title); return Ember.isEmpty(title) ? "" : Discourse.Utilities.escapeExpression(title);
}.property("notification.data.{badge_name,topic_title}"), }.property("notification.data.{badge_name,topic_title}"),
_markRead: function(){ _markRead: function(){

View File

@ -45,7 +45,7 @@ export default Em.Component.extend(StringBuffer, {
var title = Em.get(l, 'title'); var title = Em.get(l, 'title');
if (!Em.isEmpty(title)) { if (!Em.isEmpty(title)) {
title = Handlebars.Utils.escapeExpression(title); title = Discourse.Utilities.escapeExpression(title);
buffer.push(Discourse.Emoji.unescape(title)); buffer.push(Discourse.Emoji.unescape(title));
} }
if (clicks) { if (clicks) {

View File

@ -40,7 +40,7 @@ const PosterNameComponent = Em.Component.extend({
// Are we showing full names? // Are we showing full names?
if (name && this.get('displayNameOnPosts') && (this.sanitizeName(name) !== this.sanitizeName(username))) { if (name && this.get('displayNameOnPosts') && (this.sanitizeName(name) !== this.sanitizeName(username))) {
name = Handlebars.Utils.escapeExpression(name); name = Discourse.Utilities.escapeExpression(name);
buffer.push("<span class='full-name'><a href='" + url + "' data-auto-route='true' data-user-card='" + username + "'>" + name + "</a></span>"); buffer.push("<span class='full-name'><a href='" + url + "' data-auto-route='true' data-user-card='" + username + "'>" + name + "</a></span>");
} }
@ -48,7 +48,7 @@ const PosterNameComponent = Em.Component.extend({
let title = post.get('user_title'); let title = post.get('user_title');
if (!Em.isEmpty(title)) { if (!Em.isEmpty(title)) {
title = Handlebars.Utils.escapeExpression(title); title = Discourse.Utilities.escapeExpression(title);
buffer.push('<span class="user-title">'); buffer.push('<span class="user-title">');
if (Em.isEmpty(primaryGroupName)) { if (Em.isEmpty(primaryGroupName)) {
buffer.push(title); buffer.push(title);

View File

@ -29,7 +29,7 @@ export default Ember.Component.extend(StringBuffer, {
const self = this; const self = this;
const renderIcon = function(name, key, actionable) { const renderIcon = function(name, key, actionable) {
const title = Handlebars.Utils.escapeExpression(I18n.t(`topic_statuses.${key}.help`)), const title = Discourse.Utilities.escapeExpression(I18n.t(`topic_statuses.${key}.help`)),
startTag = actionable ? "a href" : "span", startTag = actionable ? "a href" : "span",
endTag = actionable ? "a" : "span", endTag = actionable ? "a" : "span",
iconArgs = key === 'unpinned' ? { 'class': 'unpinned' } : null, iconArgs = key === 'unpinned' ? { 'class': 'unpinned' } : null,

View File

@ -265,7 +265,7 @@ export default Ember.Controller.extend({
if (currentTopic) { if (currentTopic) {
buttons.push({ buttons.push({
"label": I18n.t("composer.reply_here") + "<br/><div class='topic-title overflow-ellipsis'>" + Handlebars.Utils.escapeExpression(currentTopic.get('title')) + "</div>", "label": I18n.t("composer.reply_here") + "<br/><div class='topic-title overflow-ellipsis'>" + Discourse.Utilities.escapeExpression(currentTopic.get('title')) + "</div>",
"class": "btn btn-reply-here", "class": "btn btn-reply-here",
"callback": function() { "callback": function() {
composer.set('topic', currentTopic); composer.set('topic', currentTopic);
@ -276,7 +276,7 @@ export default Ember.Controller.extend({
} }
buttons.push({ buttons.push({
"label": I18n.t("composer.reply_original") + "<br/><div class='topic-title overflow-ellipsis'>" + Handlebars.Utils.escapeExpression(this.get('model.topic.title')) + "</div>", "label": I18n.t("composer.reply_original") + "<br/><div class='topic-title overflow-ellipsis'>" + Discourse.Utilities.escapeExpression(this.get('model.topic.title')) + "</div>",
"class": "btn-primary btn-reply-on-original", "class": "btn-primary btn-reply-on-original",
"callback": function() { "callback": function() {
self.save(true); self.save(true);

View File

@ -17,7 +17,7 @@ export default Ember.Controller.extend(ModalFunctionality, {
var success = function(data) { var success = function(data) {
// don't tell people what happened, this keeps it more secure (ensure same on server) // don't tell people what happened, this keeps it more secure (ensure same on server)
var escaped = Handlebars.Utils.escapeExpression(self.get('accountEmailOrUsername')); var escaped = Discourse.Utilities.escapeExpression(self.get('accountEmailOrUsername'));
var isEmail = self.get('accountEmailOrUsername').match(/@/); var isEmail = self.get('accountEmailOrUsername').match(/@/);
var key = 'forgot_password.complete_' + (isEmail ? 'email' : 'username'); var key = 'forgot_password.complete_' + (isEmail ? 'email' : 'username');

View File

@ -75,7 +75,7 @@ export default Ember.Controller.extend({
} }
}); });
} }
return Handlebars.Utils.escapeExpression(q); return Discourse.Utilities.escapeExpression(q);
}, },
_searchOnSortChange: true, _searchOnSortChange: true,

View File

@ -76,7 +76,7 @@ Discourse.Dialect.on('parseNode', function (event) {
} else { } else {
regexp = /^ +| +$/g; regexp = /^ +| +$/g;
} }
node[node.length-1] = Handlebars.Utils.escapeExpression(contents.replace(regexp,'')); node[node.length-1] = Discourse.Utilities.escapeExpression(contents.replace(regexp,''));
} }
}); });

View File

@ -13,7 +13,7 @@ var parser = window.BetterMarkdown,
emitters = [], emitters = [],
hoisted, hoisted,
preProcessors = [], preProcessors = [],
escape = Handlebars.Utils.escapeExpression; escape = Discourse.Utilities.escapeExpression;
/** /**
Initialize our dialects for processing. Initialize our dialects for processing.

View File

@ -5,7 +5,7 @@ const Safe = Handlebars.SafeString;
export default Ember.Handlebars.makeBoundHelper(function(user, args) { export default Ember.Handlebars.makeBoundHelper(function(user, args) {
if (!user) { return; } if (!user) { return; }
const name = Handlebars.Utils.escapeExpression(user.get('name')); const name = Discourse.Utilities.escapeExpression(user.get('name'));
const currentUser = args.hash.currentUser; const currentUser = args.hash.currentUser;
if (currentUser && user.get('admin') && currentUser.get('staff')) { if (currentUser && user.get('admin') && currentUser.get('staff')) {

View File

@ -1,3 +1,18 @@
var discourseEscape = {
"&": "&amp;",
"<": "&lt;",
">": "&gt;",
'"': "&quot;",
"'": "&#x27;",
'`': '&#x60;'
};
var discourseBadChars = /[&<>"'`]/g;
var discoursePossible = /[&<>"'`]/;
function discourseEscapeChar(chr) {
return discourseEscape[chr];
}
Discourse.Utilities = { Discourse.Utilities = {
translateSize: function(size) { translateSize: function(size) {
@ -24,6 +39,28 @@ Discourse.Utilities = {
} }
}, },
// Handlebars no longer allows spaces in its `escapeExpression` code which makes it
// unsuitable for many of Discourse's uses. Use `Handlebars.Utils.escapeExpression`
// when escaping an attribute in HTML, otherwise this one will do.
escapeExpression: function(string) {
// don't escape SafeStrings, since they're already safe
if (string instanceof Handlebars.SafeString) {
return string.toString();
} else if (string == null) {
return "";
} else if (!string) {
return string + '';
}
// Force a string conversion as this will be done by the append regardless and
// the regex test will do this transparently behind the scenes, causing issues if
// an object's to string has escaped characters in it.
string = "" + string;
if(!discoursePossible.test(string)) { return string; }
return string.replace(discourseBadChars, discourseEscapeChar);
},
avatarUrl: function(template, size) { avatarUrl: function(template, size) {
if (!template) { return ""; } if (!template) { return ""; }
var rawSize = Discourse.Utilities.getRawSize(Discourse.Utilities.translateSize(size)); var rawSize = Discourse.Utilities.getRawSize(Discourse.Utilities.translateSize(size));

View File

@ -138,7 +138,7 @@ const Composer = RestModel.extend({
const postNumber = this.get('post.post_number'); const postNumber = this.get('post.post_number');
postLink = "<a href='" + (topic.get('url')) + "/" + postNumber + "'>" + postLink = "<a href='" + (topic.get('url')) + "/" + postNumber + "'>" +
I18n.t("post.post_number", { number: postNumber }) + "</a>"; I18n.t("post.post_number", { number: postNumber }) + "</a>";
topicLink = "<a href='" + (topic.get('url')) + "'> " + (Handlebars.Utils.escapeExpression(topic.get('title'))) + "</a>"; topicLink = "<a href='" + (topic.get('url')) + "'> " + Discourse.Utilities.escapeExpression(topic.get('title')) + "</a>";
usernameLink = "<a href='" + (topic.get('url')) + "/" + postNumber + "'>" + this.get('post.username') + "</a>"; usernameLink = "<a href='" + (topic.get('url')) + "/" + postNumber + "'>" + this.get('post.username') + "</a>";
} }

View File

@ -73,6 +73,8 @@
//= require ./discourse/components/topic-notifications-button //= require ./discourse/components/topic-notifications-button
//= require ./discourse/lib/link-mentions //= require ./discourse/lib/link-mentions
//= require ./discourse/views/header //= require ./discourse/views/header
//= require ./discourse/lib/utilities
//= require ./discourse/dialects/dialect
//= require ./discourse/views/composer //= require ./discourse/views/composer
//= require ./discourse/lib/show-modal //= require ./discourse/lib/show-modal
//= require ./discourse/lib/screen-track //= require ./discourse/lib/screen-track

View File

@ -5,7 +5,7 @@ module Barber
class EmberCompatPrecompiler < Barber::Precompiler class EmberCompatPrecompiler < Barber::Precompiler
def sources def sources
[handlebars, precompiler] [File.open("#{Rails.root}/vendor/assets/javascripts/handlebars.js"), precompiler]
end end
def precompiler def precompiler

View File

@ -78,9 +78,9 @@ module PrettyText
ctx_load(ctx, ctx_load(ctx,
"vendor/assets/javascripts/better_markdown.js", "vendor/assets/javascripts/better_markdown.js",
"app/assets/javascripts/defer/html-sanitizer-bundle.js", "app/assets/javascripts/defer/html-sanitizer-bundle.js",
"app/assets/javascripts/discourse/lib/utilities.js",
"app/assets/javascripts/discourse/dialects/dialect.js", "app/assets/javascripts/discourse/dialects/dialect.js",
"app/assets/javascripts/discourse/lib/censored-words.js", "app/assets/javascripts/discourse/lib/censored-words.js",
"app/assets/javascripts/discourse/lib/utilities.js",
"app/assets/javascripts/discourse/lib/markdown.js", "app/assets/javascripts/discourse/lib/markdown.js",
) )

View File

@ -64,11 +64,16 @@ var __module3__ = (function(__dependency1__) {
">": "&gt;", ">": "&gt;",
'"': "&quot;", '"': "&quot;",
"'": "&#x27;", "'": "&#x27;",
"`": "&#x60;" '`': '&#x60;',
'\n' : '\\n', // NewLine
'\r' : '\\n', // Return
'\b' : '\\b', // Backspace
'\f' : '\\f', // Form fee
'\t' : '\\t', // Tab
'\v' : '\\v' // Vertical Tab
}; };
var badChars = /[&<>"'`\b\f\n\r\t\v]/g;
var badChars = /[&<>"'`]/g; var possible = /[&<>"'`\b\f\n\r\t\v]/;
var possible = /[&<>"'`]/;
function escapeChar(chr) { function escapeChar(chr) {
return escape[chr]; return escape[chr];