3

This isn't about what html escaping is or how it's done, but if there's an established best practice about when to do it.

I have some utility code in my plugin that may generate a WP_Error based on user input, and other display code that shows that WP_Error. Of course that user input needs to be html escaped when displaying, but I'm not sure when would be the best time to do it.

I have a choice about whether to:

  • Escape the message as I'm constructing the WP_Error, and the display code shows it as-is.

  • Don't worry about escaping when constructing the WP_Error, and in the display code fully escape all the WP_Error messages.

Either would work, but if my plugin ends up interacting with other plugins and possibly displaying their WP_Error or vice-versa, I'd like to match whatever precedent exists in the Wordpress world.

I had hoped the documentation would address this, but I didn't see anything on https://codex.wordpress.org/Class_Reference/WP_Error

2

No, escaping should happen at the moment of output ( late escaping ) so that we know that it only occurs once. Double escaping can allow specially crafted output to break out.

By escaping, we're talking about functions such as esc_html, wp_kses_post, esc_url, etc.

Sanitizing functions and validating functions are not the same, e.g. sanitize_textfield. Sanitising cleans data, validation tests data, escaping enforces a type of data.

Think of it like this for a triangle factory:

  • The validator tests the item is a triangle , and gives it a thumb up or down, it only observes. If it's triangle shaped, yes! If not, no. Validators are a good way to reject input, and should happen on input.
  • The sanitizer cleans the item, removes things that don't belong on a triangle on input. Other examples might be removing trailing spaces. If the item is meant to be a number it might strip out any letters and symbols. The end result is just a cleaner, easier to manage version of what went in.
  • The escaper enforces the triangle shape, it's like a cookie cutter with an triangle shaped hole in a triangle factory. It doesn't matter what goes in, a triangle is coming out, even if a square went in. Sure you might end up with a mangled broken thing that's triangle shaped, but triangles are what we intended and triangles are what we're getting

Double escaping is bad because if we then take our broken square that's shaped like a triangle, flip it upside down, and run it through the cookie cutter a second time, we don't get a triangle, we get a hexagon. We didn't want hexagons!

We can sanitize and validate as many times as we want, but we should only escape once, and it should happen at the moment of output, if not as close as possible to it.


So since WP_Error does not output, and is not responsible for outputting, it should not perform any escaping internally, nor should its inputs be escaped. Validated/sanitized perhaps, but not escaped.

If we did escape on input, we would either have to double escape, or trust all WP_Error objects, which is a non-starter. All it would take is a single plugin developer forgetting to escape on input. It's also a lot easier to escape in the one place that outputs, than to escape in the many places that input.

So instead, the code that receives and outputs the WP_Error object is where the escaping should be, that way we escape on output safe in the knowledge that no early escaping has occurred, no double escaping happens, whether it's been escaped is not a problem we have to deal with, and the responsibility for escaping is clear and straight forward

As for how to safely output a WP_Error object, either esc_html or wp_kses_post will suffice. The latter will allow tags such as <strong> etc. Generally, putting anything more complex in an error object than what you'd put into a post is a bad idea, WP_Error objects are structured data already

  • "or trust all WP_Error objects, which is a non-starter" This essentially answers my question. Definitely sounds like WP_Error cannot be assumed to contain html escaped content, therefore WP_Errors MUST be escaped when displayed. – Jason Viers Jan 14 at 3:57
  • And what with error messages that contain HTML tags? You can’t escape all messages during printing them - even WP doesn’t escape them during printing... – Krzysiek Dróżdż Jan 14 at 7:30
  • 1
    WP does escape in a lot of cases, and wp_kses and wp_kses_post are useful here when outputting. Eitherway you shouldn't be inserting styling into error messages, it's the responsibility of the code that renders the error object to do that. Perhaps at most a pre tag or a strong but nothing that couldn't appear in a post anyway – Tom J Nowell Jan 14 at 16:43
  • Thanks for mentioning wp_kses and wp_kses_post, that will be very useful and take all the sting out of the "escape at display time" method! – Jason Viers Jan 14 at 17:39
2

It's a very good question.

WP_Error class doesn't do anything with messages you set. So what you set is what you get ;)

On the other hand, the error messages can contain HTML, so you can't escape all of them, when printing errors. For example here you get a message from wp-login.php file:

$errors->add('empty_username', __('<strong>ERROR</strong>: Enter a username or email address.'));

And to make things even harder - as you can see - it's passed through __() function, so after translation the message may contain even more HTML tags.

All of that means that you should take care of escaping data that may be harmful and remember that errors can contain HTML tags when printing them.

  • This was the difficulty I was imagining - if my display code somehow ended up getting a hold of the WP_Error with that add()-ed message, it will display literal <strong> tags to the user. But it sounds like it's unsafe to assume that tags in WP_Error are "intentional from devs and safe", so it'll have to be escaped on display. – Jason Viers Jan 14 at 3:59
  • @JasonViers on the other hand, as you can see, some tags are intentional - even in WP core. – Krzysiek Dróżdż Jan 14 at 10:12
2

Foreword: My answer is not really an answer to your specific question but it's a vital, important thing to know that ties into how most people I know use error strings: display the translated error messages to the end-user.

I can't add much more on what Tom already said, the answer is no to your original question, as I'll repeat myself at the end of this answer, a WP_Error message is a pure string that should hold nothing but simple characters, ready to be used in any system. Putting markup or such in them defeats their purpose, but there's a big thing you probably don't know.

Translation & display of that translation.

Let's face it - error messages are meant to be displayed and most likely translated, depending on your product, but most plugins & themes do this, I can come up with 20+ use cases where error display is crucial. Where do the issues start?

The culprit is Polylang, but it's not its fault, every translation plugin will behave like this, they will never escape their translated strings. On their screens and how they store it, sure, that's the plugins' job, but what you do with the string after the plugin's given it to you - your job.

So, how does it work?

Integrate a string with Polylang, but instead of a normal string, put this in:

<body onload=alert('XSS')>

Now, when you echo this translated string, wherever, what will happen is, you get a popup:

enter image description here

If you were to just escape it, this would never be an issue. The problem here is that if you gave translate rights to users or there's a hole with the plugin itself somewhere else that allows users to launch a "modify translate string", your site is hacked. In fact, if you have your translation files up and someone inserts any JS and you don't output it correctly and you accept that translation and it goes to the repository, anyone that uses that translation package will have malicious JS on their site, this will probably never happen but just telling you.

Back on track, whenever you output a translated string, it'll give you whatever whoever translated that string did:

echo fav_plugin_output_translated_string( 'string-1' ) // will output whatever matches

which could be malicious, as such, if the plugin doesn't have an in-built escape function, do, depending on where you need that string, but it's 99% of the cases for display within HTML:

echo esc_html( fav_plugin_output_translated_string( 'string-1' ) );

this way, no matter what a malicious user put in, you'll always be safe, or well, as safe as esc_html is supposed to be.

Remember, when you output any translated strings, never trust them, escape and perhaps leave a way for anyone interacting with your system to turn off that escaping in case they wanna do something with it.

Your theme / plugin is your system, even if it's very extendable, it can never be extended without creating another plugin and what that does - not your job, you did your part and made sure that whenever you output strings or store them for display later on, you correctly escaped them.

There's no way you can control the validator / sanitizers / cleaners of others', but you can control how the escape happens for display.

1

Personally, I'd keep the error messages basically plain text and static (don't include the user's input) - from what I've seen, most plugins take this mindset of error messages being short text-only notes of what has gone wrong. This allows you to use messages like "Password is too short" vs "'MyPass' is to short of a password".

If you need some HTML in the error message itself, I'd escape it on the way in.

$error = new WP_Error();

$message = "The correct tag is <strong></strong>";
$html_ok_message = htmlspecialchars($message);

$error->add($code, $html_ok_message, $data);

You could escape them on the way out if you know all the possible errors encountered have no HTML that needs to be displayed. If you escape them all always when outputting, you'll likely end up with things like &lt;strong;Error&gt; Something went wrong in XY_Other_plugin .

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.