kallithea Changeset - 603f5f7c323d

Changeset - 603f5f7c323d

Parent rev.

Child rev.

[Not reviewed]

stable

0 2 0

Thomas De Schampheleire - 7 years ago 2019-02-26 21:50:15
thomas.de_schampheleire@nokia.com

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).

2 files changed with 12 insertions and 1 deletions:

kallithea/public/js/base.js

kallithea/templates/pullrequests/pullrequest_show.html

0 comments (0 inline, 0 general)

kallithea/public/js/base.js

➞

Show inline comments

@@ @@ -1310,12 +1310,14 @@ var MentionsAutoComplete = function ($in @@
             mentionsAC.dataSource.chunks = unam[1];
             mentionsAC.dataSource.mentionQuery = curr_search;
         });
+}
 // 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 != "")) {
         displayname = "{0} {1} ({2})".format(fname, lname, nname);
+    }
     var gravatarelm = gravatar(gravatar_link, gravatar_size, "");
@@ @@ -1369,12 +1371,14 @@ var PullRequestAutoComplete = function ( @@
     if(reviewerAC.itemSelectEvent){
         reviewerAC.itemSelectEvent.subscribe(function (sType, aArgs) {
             var myAC = aArgs[0]; // reference back to the AC instance
             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 = '';
         });
+    }
+}

kallithea/templates/pullrequests/pullrequest_show.html

➞

Show inline comments

@@ @@ -409,13 +409,20 @@ ${self.repo_context_bar('showpullrequest @@
           if (window.location.hash != "") {
               window.location.href = window.location.href;
+          }
           $('.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>
 </div>

0 comments (0 inline, 0 general)