0

Is the correct strategy to never escape data before it's put into a database. I feel like I've seen that referenced before but I couldn't find it now and this didn't clear it up for me:

https://codex.wordpress.org/Validating_Sanitizing_and_Escaping_User_Data

This came up because I'm trying to search a field and A&R becomes A&R so I have two choices:

  1. I can HTML encode (esc_html) search input
  2. I can not HTML encode the field when it's inserted into the database (and only escape / encode on output)

I see wp_posts.post_title is not escaped and wp_terms.name is so even WordPress proper is internally inconsistent on this. What is the proper approach?

3

You escape on output, what I suspect here is a confusion between escaping sanitizing and validating

  • Sanitise when data arrives. This strips out stuff that shouldn't be there, e.g. upper case letters in a lower case string, words and letters in a phone number, trailing spaces etc. Sanitising cleans data
    • common sanitising functions include trim, sanitize_key, wp_strip_tags, intval, wp_kses_post, etc
  • Validate data once it's been sanitised. Is that phone number really a phone number? Did somebody say they were -20 years old? Why does that mans address describe the recipe for cheese pasties? If it doesn't validate, REJECT the data. If it does, process then store the value.
    • common validation includes regular expressions, is_numeric, checking string length, is_email, etc, validation usually requires context specific checks, e.g. enforcing values sit within a range, or that they appear in a whitelist array
  • Escape when you're outputting that saved data to the frontend. Do this once and only once, at the moment of output. Don't return complex HTML fragments in functions, echo them, and escape when you echo. Escaping might mangle your output if unsafe stuff makes it through, but it guarantees that the URL is always a URL, even if it's a broken URL, that the text is always text, even if it contained a dangerous tag, etc
    • Escaping functions include, but aren't limited to esc_html, esc_attr, esc_url, wp_kses_post, etc

Note that sanitising and validation happen when data is incoming, data that came from an external source such as the browser.

Escaping happens when data is being sent, to an API/user/browser. Escaping is like the cookie cutter that enforces expectations. If the variable contains a HTML attribute, esc_attr will ensure it's an attribute, and doesn't contain anything that's invalid for an attribute, even if that means mangling the value to make it fit, removing bits. Of course, if you've escaped correctly, valid values will never be mangled.

Note that if you escape a value more than once, it can allow carefully crafted data to break out and render malicious output in some circumstances. This is why you only escape on output, and always as late as is possible. Pre-escaped data introduces the headache of having to track what has and hasn't been escaped, aint nobody got time for that.

With escaping, trust is unnecessary, you should never have to trust that something is safe when you can force it to be safe.

  • So sanitizing data before DB insertion is fine but escaping is not? To take a couple concrete examples all of the following would be fine to run before inserting as they sanitize instead of escape: 1) sanitize_post_field for the post_title instead of esc_html as I was doing. 2) esc_url_raw instead of esc_url 3) a field with HTML content wp_kses or wp_strip_all_tags? Do you have a link / reference for double escaping content causing security issues? It sounds interesting. – JSP Mar 6 '18 at 13:36
  • Escaping is not sanitising, you escape on output, at the latest possible moment. Right at the very last moment. 1) No, sanitizing and escaping do different things, escape on output, sanitize on input 2) It depends on the context, esc_url is almost always the correct answer 3) It depends, wp_kses_post should do, but if your HTML contains non-content markup such as iframes, link, script tags, then it's impossible to escape and you've made a mistake in the way your code is structured. wp_strip_all_tags is a sanitizing mechanism, apply esc_html afterwards – Tom J Nowell Mar 6 '18 at 15:02
  • and nope i'd have to google for a resource on double escaping, so i'll leave that as a task for you, but it involves escaping escape characters, it's not a good idea to double escape, for many practical reasons – Tom J Nowell Mar 6 '18 at 15:02
0

To add to @tom's answer regarding core code. Core code is ancient in software terms and was done by different people with different expertise level, but the more important thing is that once the functionality is "in the wild" it is very hard to fix it without breaking plugin and themes that might rely on the particular way things work.

Point in case, widget titles are being aggressively sanitized and are not being escaped. This is a "prehistoric" code, and a behaviour some plugins actually depend on.

So.... when you write your own code, follow what @Tom said, when you need to use core API, and you are not sure what it does regarding escaping/sanitation, just look at the code itself.

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.