Fix a XSS security regression with rendering profile display names.
Review Request #1451 — Created Nov. 9, 2018 and updated
Information | |
---|---|
guest6813 | |
Review Board | |
67c738d... | |
Reviewers | |
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/