New role name not filtered into admin/user/permissions

This should be fixed, but is not really a security hole - to add new roles requires 'administer permissions' which already means you essentially can escalate your own permissions without limit.

Catching it in the user_roles() function also gets most places it's displayed - but might lead to double-encoding.

This will break any select lists using the output of the user_roles() function, so guess we would need to track down uses.

However, it would perhaps make more sense to just validate on input as we do for translations.

Right. We're back to the evil checkboxes are output unescaped, while select lists are escaped by FAPI. Ugh. Given that, I think we really do have to continue to output potentially malicious text from user_roles(), and check the call sites 1-by-1 to fix this.

I think it's reasonable to consider validation-on-input, too, but then it seems like we need a DB update to check for existing roles that aren't valid. Not sure what exactly to do with them during the DB update in that case. :( Given that, we should probably stick with escape-on-output...

While we're grepping, we should also make sure no one does anything stupid with $user->roles directly. The hunk in user_edit() is the only spot I can think of where we do anything with that, but there might be others, so we should check.

I'm *really* tempted to call this "minor". If you can't trust the users that have 'administer permissions', you're already in deep trouble.

Is check_plain the right filter here? The issue we're trying to fix is XSS. Why not filter_xss_admin?

Validation on input goes against our overall filtering practice. It also won't save us when some contrib/install profile creates a role name based on user input. We filter on output.

Because rolenames are plaintext (would you expect tags in them? (no) Ampersands ? (yes)), they need to be check_plained to get even valid HTML.

Also from Handle text in a secure fashion:

Plain-text
This is simple text without any markup. What the user entered is displayed exactly on screen as is, and is not interpreted in any form. This is generally the format used for single-line text fields.

When outputting plain-text, you need to pass it through check_plain() before it can be put inside HTML. This will convert quotes, ampersands and angle brackets into entities, causing the string to be shown literally on screen in the browser.