Prevent XSS via form elements, consider removing extra check_plain [#1127788]

The documentation about handling text in a secure fashion mentions that form #values need to be filtered if the #type is 'value' because value is assumed to be html.

You should run check_plain before displaying user submitted data (like usernames, email addresses) on the form.

Consider whether the username should actually be presented with theme('username', array('name' => $user->name)); instead of just printed.

I'm not sure that check_plain is right for all of the variables in the og_email_blast_mail. Especially if they will be check_plained in the form generation process they shouldn't be double filtered.

Comments

Comment #1

dcmouyard commented 18 April 2011 at 14:50

Status:

Active

» Fixed

I filtered the form #values with check_plain for group, admin, and admin_email. I didn't use theme(username) for admin because it's only displayed as plain text in the email and the watchdog message.

I also filtered the result of _group_emails() with check_plain.

Finally, I removed the redundant check_plain filters in og_email_blast_mail(), except for subject and body, since they haven't been filtered yet.