Fix a XSS security regression with rendering profile display names.

Review Request #1451 — Created Nov. 9, 2018 and updated

guest6813
Review Board
67c738d...
guest5837
The work done this year to secure the full names of users by default
introduced a XSS vulnerability. If a user had a full name set to
something malicious, and the requesting user had access to their
profile, the malicious content would be injected anywhere the display
name was being used. This came up in the datagrids (Dashboard, Users
page, etc.) and in avatar rendering.

The reason this happened is that the new datagrid `FullNameColumn` and
`{% user_profile_display_name %}` template tag trusted that the display
name was HTML-safe. They expected another layer to handle escaping the
content. While newer versions of Django would handle escaping the
template tag by default, and Djblets 2.0 will handle escaping the
rendered column data, the data today was not escaped.

The fix is to explicitly escape these calls before returning the values
to the template.

Testing Done:
Unit tests pass.

Manually verified the fixes.

Grepped around the codebase looking for other places where
`get_display_name()` was called, and tested each to check for
vulnerabilities.

Reviewed at https://reviews.reviewboard.org/r/10279/


  • 1
  • 0
  • 1
  • 1
  • 3
Description From Last Updated
review from guest6813 guest6813 guest6813
guest6813
Review request changed

People:

-guest
+guest5837
guest5837
  1. 
      
  2. reviewboard/accounts/tests/test_template_tags.py (Diff revision 1)
     
     
     
     

    non-issue comment from guest5837

  3. reviewboard/datagrids/columns.py (Diff revision 1)
     
     
     
     
     
     
     
     
     

    comment from guest5837

  4. 
      
guest6813
  1. 
      
  2. reviewboard/accounts/templatetags/accounts.py (Diff revision 1)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     

    review from guest6813

  3. reviewboard/datagrids/columns.py (Diff revision 1)
     
     

    comment from guest6813

  4. 
      
guest8166
  1. Ship It!
  2. lkjlk

  3. 
      
guest8166
  1. 
      
  2. ;lkk;l

  3. 
      
Loading...