# HG changeset patch # User Mads Kiilerich # Date 2019-02-27 02:23:26 # Node ID 9beef1d91c4cd2455d2d796eec19f8a068a4468c # Parent 22da5f2581183b5d085b8d0355e4bc852b988fba pullrequests: prevent XSS when 'Potential Reviewers' are selected and first and last names cannot be trusted The user information passed to autocompleteFormatter from select2 is the raw data which might contain HTML markup controlled by the user. That could cause XSS issues, already when adding rogue users as reviewers on a PR. To avoid that, make sure select2 use the default escapeMarkup function. In addReviewMember, use .html_escape when expanding the reviewer template. diff --git a/kallithea/public/js/base.js b/kallithea/public/js/base.js --- a/kallithea/public/js/base.js +++ b/kallithea/public/js/base.js @@ -1146,7 +1146,6 @@ var SimpleUserAutoComplete = function ($ }, formatSelection: autocompleteFormatter, formatResult: autocompleteFormatter, - escapeMarkup: function(m) { return m; }, id: function(item) { return item.nname; }, }); } @@ -1172,7 +1171,6 @@ var MembersAutoComplete = function ($inp }, formatSelection: autocompleteFormatter, formatResult: autocompleteFormatter, - escapeMarkup: function(m) { return m; }, id: function(item) { return item.type == 'user' ? item.nname : item.grname }, }).on("select2-selecting", function(e) { // e.choice.id is automatically used as selection value - just set the type of the selection @@ -1249,7 +1247,7 @@ var addReviewMember = function(id,fname, ' (add not saved)\n'+ ' \n'+ ' \n' - ).format(gravatarelm, displayname, id); + ).format(gravatarelm, displayname.html_escape(), id); // check if we don't have this ID already in var ids = []; $('#review_members').find('li').each(function() { @@ -1289,7 +1287,6 @@ var PullRequestAutoComplete = function ( }, formatSelection: autocompleteFormatter, formatResult: autocompleteFormatter, - escapeMarkup: function(m) { return m; }, }).on("select2-selecting", function(e) { addReviewMember(e.choice.id, e.choice.fname, e.choice.lname, e.choice.nname, e.choice.gravatar_lnk, e.choice.gravatar_size);