Changeset - 603f5f7c323d
[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:
0 comments (0 inline, 0 general)
kallithea/public/js/base.js
Show inline comments
 
@@ -1313,6 +1313,8 @@ var MentionsAutoComplete = function ($in
 
        });
 
}
 

	
 
// 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 PullRequestAutoComplete = function (
 
            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
 
@@ -412,7 +412,14 @@ ${self.repo_context_bar('showpullrequest
 

	
 
          $('.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>
0 comments (0 inline, 0 general)