Justin C. Klein Keane reported that content_access does not check_plain role names before displaying them on the 'Access Control' screen of managed content types.
We do not consider this a vulnerability because this can only be exploited by users with the "administer permissions" permission. As users with the "administer permissions" can already elevate their permissions via normal means, an XSS attack is just a more complicated way of doing something that is already possible.
It is a bug however; As role names are plaintext, they cannot be passed as-is in HTML. If you do not check_plain rolenames you may end up with invalid HTML or break something (& vs. &
).
Comment | File | Size | Author |
---|---|---|---|
#4 | content_access_6.x-1.1.patch | 447 bytes | Justin_KleinKeane |
#3 | content_access_5.x-1.5.patch | 500 bytes | Justin_KleinKeane |
Comments
Comment #1
pwolanin CreditAttribution: pwolanin commentedIs this duplicate with this? #316136: New role name not filtered into admin/user/permissions
If not, it's certainly related.
Comment #2
Heine CreditAttribution: Heine commentedRelated yes, duplicate no, because this is about output from content access.
Comment #3
Justin_KleinKeane CreditAttribution: Justin_KleinKeane commentedAttached patch should address the issue in 5.x-1.5. Thanks.
Comment #4
Justin_KleinKeane CreditAttribution: Justin_KleinKeane commentedAttached patch should address the issue in 6.x-1.1. Thanks.
Comment #5
Les LimChanging status from active to patch needs work. Haven't tried the patches, but they need to be cleaned up according to Drupal coding standards for control structures.
Comment #6
apadernoI agree they should be changed to follow the Drupal coding standards; in the specific case, the block of code executed from the foreach statement should be in a new line.
Comment #7
fagoAs core doesn't check_plain() either I don't think it makes much sense doing so in contrib. So I just fixed it to go through filter_xss_admin().
Comment #8
Heine CreditAttribution: Heine commentedIf core makes a mistake (see #316136: New role name not filtered into admin/user/permissions), there's no reason to repeat it. Rolenames are not HTML and need to be converted by check_plain to get valid HTML.
Comment #9
fagoI agree with you that 'check_plain' would be the best option, however as we discussed in IRC I think the strings should be handled in a consistent way. Thus I use filter_xss_admin() just to be safe but follow the (bad) way of core to display HTML. If core changes to use check_plain() I'm happy to follow that.