CSS classes for WP List Table are not properly sanitized

[sbressler]

The output of a WP List Table uses the singular and plural names of the table (provided as arguments to the constructor) as the CSS classes output for the table and tbody. Those names aren't sanitized at all. The biggest concern I have is that spaces aren't turned into dashes, though the lack of sanitization is bad practice in general.

[sbressler]

Patch that sanitizes the title of the constructor args

[sbressler]

Attached (18875.1.diff​) is a patch that fixes this issue. I decided to address the issue in the constructor rather than before the classes were output so that we can address any other uses of $args that might be added later to the file.

While $args['singular'] might be empty, there is no harm to running it through sanitize_title as well. However, that line could easily be wrapped in if ( $args['singular'] ) if performance is a concern.

[nacin]

Can we switch this out for sanitize_html_class()?

What could this break? What are the current inputs for singular/plural?

[sbressler]

sanitize_html_class doesn't change spaces into dashes, which sanitize_title does. While we can leave that to be the responsibility of users of WP_List_Table, I think using dashes for replacements makes more sense.

I need to check all the users of WP_List_Table to see if this would affect any of them. My guess is that it won't, but I'll verify later today.

[nacin]

I'm not entirely familiar with how these are used, but it doesn't look like they're just used strictly as classes. Perhaps sanitize_key() would be the best choice, and it wouldn't break anything in core (best I can tell - these are tough to search for).

[sbressler]

Finally got around to investigating this. $_args['plural'] is used as a CSS class (in the base class as well as the plugins list class) and as a nonce. $_args['singular'] is used only as a CSS class (in the base class). In all cases, I think converting spaces to dashes make the most sense. As such, I think sanitize_title makes the most sense.

However, if performance is a significant concern with using sanitize_title(), using sanitize_html_class() or sanitize_key() would be fine.

[nacin]

In [19020]:

Sanitize plural and singular args for list tables. props sbressler, fixes #18875.

[nacin]

If nonces or CSS classes break, there's no true damage. A plugin would A) need to be instantiating a list table with some weird non-key values, and B) trying to target said weirdness with CSS, for sanitize_key() to break something. I'd rather go with this than a sledgehammer.