Escape output in settings_errors

[tollmanz]

The Problem

The "settings_errors" function does not escape data when outputting it from the $settings_errors variable, which either comes from the $wp_settings_errors global variable or the "settings_errors" transient. This data is not escaped at any point during retrieval or output. Additionally, the data is not sanitized or validated when adding it via "add_settings_error".

Test Case

In the validation callback function for a setting, adding a settings error with HTML can badly break output:

add_settings_error( 'zdt-setting', '1023', 'An error occurred</div>' );

Solution

Escape the $type, $code, and $message variables on output.

Possible Issues

The $message variable is output wrapped in a 'p' and 'strong' tag. My patch tries to allow common, reasonable tags to be output. I use "wp_kses_data", which will only allow tags defined in the $allowedtags variable. Should a plugin or theme author need another element printed out, it will be stripped; however, there still is the potential that it could cause issues for plugins that were previously able to place anything in the $message variable. With that said, most other tags would lead to invalid HTML and probably shouldn't be allowed in this context anyway.

Here's the way I'm reading this code / documentation: add_settings_error is expecting $message to contain pre-sanitized data and to be formatted exactly as the theme developer intends.

The docs say this:

 * @param string $message The formatted message text to display to the user (will be shown inside styled <div> and <p>)

I would argue that any user data being passed through this function should already be sanitized.

add_settings_error( 'zdt-setting', '1023', 'The value: ' . wp_kses_data( $val ) . ' is not allowed' );

Otherwise WordPress should dump this data out directly.

Replying to tollmanz:

My patch tries to allow common, reasonable tags to be output. I use "wp_kses_data", which will only allow tags defined in the $allowedtags variable.

Generally WordPress does not restrict plugins in this manner - IMO that's what makes the plugin API so powerful. I think it's the responsibility of plugin authors to make sure their code doesn't break things.

Sorry for the noise.