Opened 12 years ago
Closed 8 years ago
#20125 closed enhancement (worksforme)
Escape output in settings_errors
Reported by: | tollmanz | Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | 3.0 |
Component: | Plugins | Keywords: | has-patch |
Focuses: | administration | Cc: |
Description
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.
Attachments (1)
Change History (9)
#2
in reply to:
↑ description
@
12 years ago
- Keywords close added
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.
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:
I would argue that any user data being passed through this function should already be sanitized.
Otherwise WordPress should dump this data out directly.