Mercurial > kallithea

changeset 7550:603f5f7c323d stable

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
pullrequests: prevent XSS in 'Potential Reviewers' list when first and last names cannot be trusted If a user first or last name contains javascript, these fields need proper escaping to avoid XSS attacks. An example scenario is: - the malicious user creates a repository. This will cause this user to be listed automatically under 'Potential Reviewers' in pull requests. - another user creates a pull request on that repository and selects the suggested reviewer from the 'Potential Reviewers' list. Reported by Bob Hogg <wombat@rwhogg.site> (thanks!). Technical note: the other caller of addReviewMember in base.js itself does _not_ need to be adapted to escape the input values, because the input values (oData) are _already_ escaped (by the YUI framework).
author Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
date Tue, 26 Feb 2019 21:50:15 +0100
parents fa3e6eda9e7c
children 81db5704b285
files kallithea/public/js/base.js kallithea/templates/pullrequests/pullrequest_show.html
diffstat 2 files changed, 12 insertions(+), 1 deletions(-) [+]
line wrap: on
line diff
--- a/kallithea/public/js/base.js	Mon Feb 11 21:36:13 2019 +0100
+++ b/kallithea/public/js/base.js	Tue Feb 26 21:50:15 2019 +0100
@@ -1313,6 +1313,8 @@
         });
 }
 
+// Important: input arguments to addReviewMember should be safe (escaped) for
+// inclusion into HTML.
 var addReviewMember = function(id,fname,lname,nname,gravatar_link,gravatar_size){
     var displayname = nname;
     if ((fname != "") && (lname != "")) {
@@ -1372,6 +1374,8 @@
             var elLI = aArgs[1]; // reference to the selected LI element
             var oData = aArgs[2]; // object literal of selected item's result data
 
+            // The fields in oData have already been escaped by the YUI
+            // framework. We thus should not add explicit escaping here anymore.
             addReviewMember(oData.id, oData.fname, oData.lname, oData.nname,
                             oData.gravatar_lnk, oData.gravatar_size);
             myAC.getInputEl().value = '';
--- a/kallithea/templates/pullrequests/pullrequest_show.html	Mon Feb 11 21:36:13 2019 +0100
+++ b/kallithea/templates/pullrequests/pullrequest_show.html	Tue Feb 26 21:50:15 2019 +0100
@@ -412,7 +412,14 @@
 
           $('.missing_reviewer').click(function(){
             var $this = $(this);
-            addReviewMember($this.attr('user_id'), $this.attr('fname'), $this.attr('lname'), $this.attr('nname'), $this.attr('gravatar_lnk'), $this.attr('gravatar_size'));
+            addReviewMember(
+                $this.attr('user_id'),
+                $this.attr('fname').html_escape(),
+                $this.attr('lname').html_escape(),
+                $this.attr('nname').html_escape(),
+                $this.attr('gravatar_lnk'),
+                $this.attr('gravatar_size')
+            );
           });
       });
     </script>