Steps to reproduce:

  1. Create a Drupal 7 site with the name "Somebody's site".
  2. As an administrator, create a new user account and check the box to send an e-mail notifying them of the account.
  3. Read your e-mail. The message subject in your inbox will say An administrator created an account for you at Somebody's site.

The token substitution code in _user_mail_text() results in the token replacements being sanitized, so that check_plain() runs on them. As far as I know, this shouldn't be necessary at all; in the case of the message subject it results in the double-encoding shown above. In the case of the e-mail body, the default mail implementation runs http://api.drupal.org/api/function/drupal_html_to_text/7 on that before sending it, so we actually want the HTML tags to remain in there as is.

CommentFileSizeAuthor
#1 user-token-sanitize-938536-1.patch840 bytesDavid_Rothstein

Comments

David_Rothstein’s picture

Status: Active » Needs review
StatusFileSize
new840 bytes

Here is the proposed patch. I think this makes sense. Not sure if we also want to change the function documentation to explicitly note that the text is unsanitized (in the unlikely event that someone is using the filtered output of this function for something other than an e-mail message).

JacobSingh’s picture

Priority: Normal » Major
Status: Needs review » Reviewed & tested by the community

This is a pretty ugly bug that lots of users will see and it makes Drupal look really unprofessional.

I bumped the priority to major since I think it should go out in an initial release and RTBC'd.

tom_o_t’s picture

#1: user-token-sanitize-938536-1.patch queued for re-testing.

dave reid’s picture

As long as the e-mail is sent plain text, this looks good to me!

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Yeah, that's pretty ugly.

Committed to HEAD. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.