image.admin.inc: new effect options are double escaped (check_plain)

The check_plain for the options is indeed redundant, however, it should be check_plain'd in the table row imo, because there's a potential (although small) issue here. Open up the image.effects.inc and change the label of image resize to this:

<script>alert(\'hi\')</script>

Select that effect and you'll see a nice javascript popup, so a potential xss issue (although small, as this requires the right permission, but still). Patch attached adds check_plain on the table row so this doesn't happen.

Wrong status, sorry

Still valid, rerolled since it didn't apply anymore.

Assigning to quicksketch for review

Looks good to me at first pass. I'll apply and test this evening. I'm guessing this affects 7.x too?

Select that effect and you'll see a nice javascript popup, so a potential xss issue (although small, as this requires the right permission, but still). Patch attached adds check_plain on the table row so this doesn't happen.

Agreed. This isn't an XSS hole in itself, there would need to be a module installed that specifically included an image effect that included an XSS string as an image effect label. If a module wanted to cause a ruckus though, it certainly has a lot more opportunities than XSS, since it's already running on the server anyway. ;)

Patch works as advertised, both properly escaping the image effect label when displayed in the table and not double-escaping the effect label when displayed in the "Add new effect" select list.

Committed to 8.x. Moving it to 7.x. Thanks guys.

Backported #4 to D7.

I have tested this on a clean Drupal 7.101 and it does not seems to be an issue. Closing as Fixed for D8, so that credits are assigned correctly. Thanks!