While investigating T166997 I registered an account called Foo" onmouseover="alert('XSS')" bar=" on my Labs test wiki (currently running MW 1.28 w/ latest master SP) and posted a message on my main account's (Jack Phoenix) user board via the board shown on User:Jack Phoenix -- the message was posted but the alert was also displayed.
Description
Description
Details
Details
Related Objects
Related Objects
- Mentioned Here
- T166997: XSS in MediaWikiChat extension
Event Timeline
Comment Actions
Proposed and tested fix, CR welcome:
diff --git a/UserBoard/UserBoardClass.php b/UserBoard/UserBoardClass.php index 3fa9367..cfd6175 100644 --- a/UserBoard/UserBoardClass.php +++ b/UserBoard/UserBoardClass.php @@ -407,16 +407,17 @@ class UserBoard { # $message_text = preg_replace_callback( "/(<a[^>]*>)(.*?)(<\/a>)/i", 'cut_link_text', $message['message_text'] ); $sender = htmlspecialchars( $user->getFullURL() ); + $senderTitle = htmlspecialchars( $message['user_name_from'] ); $output .= "<div class=\"user-board-message\"> <div class=\"user-board-message-from\"> - <a href=\"{$sender}\" title=\"{$message['user_name_from']}\">{$message['user_name_from']}</a> {$message_type_label} + <a href=\"{$sender}\" title=\"{$senderTitle}\">{$message['user_name_from']}</a> {$message_type_label} </div> <div class=\"user-board-message-time\">" . wfMessage( 'userboard_posted_ago', $this->getTimeAgo( $message['timestamp'] ) )->parse() . "</div> <div class=\"user-board-message-content\"> <div class=\"user-board-message-image\"> - <a href=\"{$sender}\" title=\"{$message['user_name_from']}\">{$avatar->getAvatarURL()}</a> + <a href=\"{$sender}\" title=\"{$senderTitle}\">{$avatar->getAvatarURL()}</a> </div> <div class=\"user-board-message-body\"> {$message_text}
Comment Actions
So this indeed fixes the issue this ticket is about (XSS in social profile pages)...except the same issue is also present on the Special:UserBoard special page, and this patch does not fix that, because for whatever incomprehensible reason the special page duplicates UserBoard::displayMessages(). At a glance there doesn't seem to be much difference between them, so I'm wondering if we could improve the UserBoard class method and kill the unwanted code duplication while also fixing this security issue; if not, we can just slap the htmlspecialchars() calls onto the special page code and call it a day.