esc_textarea() for obvious textarea escaping function.

[markjaquith]

Alex King brought this up privately (in case it had security implications).

We escape data in textareas a bunch of different ways. Let's introduce esc_textarea() and standardize around that.

[scribu]

I'm +1 for the idea.

[scribu]

...although I thought we used esc_html() everywhere.

[nacin]

This missed the ticket:

(In [16431]) esc_textarea() and application for obvious textarea escaping. props alexkingorg. fixes #15454

No, esc_html() isn't proper here, because we need to double-encode. The best function we had for it was wp_htmledit_pre(), or a direct htmlspecialchars() call.

[nacin]

[16485]

[scribu]

Related: #14268

[nacin]

I found another instance of us double-escaping something that somehow hasn't been noticed yet. I think we should audit each of these situations by hand to ensure we're not accidentally double-escaping.

[nacin]

#15505, [16502]

[nacin]

(In [16744]) Don't double-escape user description. see #15454.

[markjaquith]

(In [16993]) Use ENT_QUOTES in esc_textarea() in case someone uses it in a value= situation by accident. see #15454

[nacin]

(In [16995]) The user description field should be esc_textarea when context is edit. see #15454.

[garyc40]

fixed various double escaping issues

[garyc40]

two more instances

[garyc40]

There's this weird thing with sanitize_bookmark_field() and sanitize_term_field().

Originally, when $context = 'edit', they will run 'link_notes' and 'term_description' through format_to_edit($value). The 2nd parameter of format_to_edit() is left to default (which is false), which means format_to_edit($value) will escape the $value.

However, 'link_notes' and 'term_description' are already escaped before being inserted into the database (when $context = 'db'), because they're processed by "pre_link_notes" and "pre_term_description" filters, to which wp_filter_kses() is attached.

As a result, compounded with the effect of esc_textarea, these values are "triple-escaped".

I removed "format_to_edit()" from sanitize_bookmark_field() and sanitize_term_field(). However, this needs a sanity check.

[garyc40]

In current trunk, try entering this in Link Notes (Links -> Add Link), or Category Description (Post -> Categories -> Edit a category), or Biography Info (profile.php):

Test String < Hello

It will become this inside the textarea:

Test String < Hello

In the source code:

Test String < Hello

This is probably the rationale behind esc_html() in sanitize_user_field() in the first place.

[garyc40]

revert to esc_html() for link_notes, user_description and term_description

[ryan]

(In [17001]) Remove some unnecessary esc_textarea() calls. Props garyc40. see #15454

[ryan]

Partial commit of rev3. I'll look at the remainder a little later. I think the htmlspecialchars() on selection in press-this.php was added for XSS reasons.

[ryan]

For 3.1, let's limit this to fixing areas broken by the esc_textarea() commit. Let's leave the calls to format_to_edit() alone until 3.2.

[nacin]

(In [17141]) Tag textareas escaped earlier with textarea_escaped. see #15454.

[nacin]

(In [17142]) Revert [16995] due to the way the data enters the db. props garyc40, see #15454.

[nacin]

Alright. Just went through and audited the three dozen or so esc_textarea calls.

There are three textareas we're still escaping farther up the stack. In wp-admin/includes/nav-menu.php, there is a menu item description. Reverting to esc_html() handles everything except &, so that's exactly what I'm doing.

Indeed there is a problem with both link_notes and term_description, but this is not a regression from 3.0. No one has noticed, so I'm inclined to punt and do a better audit of what's going on there (both into the DB and out again) in 3.2.

[nacin]

(In [17143]) Revert to esc_html when escaping the textarea for nav menu item description. see #15454.

[nacin]

Hack to handle link_notes and term_description, from garyc40's 3rd patch. NOT a regression.

[nacin]

I'm satisfied that there are no more regressions caused by [16431].

Everything in garyc40-15454-rev3.patch is either handled by a recent commit or 15454.diff​. Or, it's in press-this.php, which there's no need to touch. (We've broken press-this enough this cycle.)

Leaving open for final review by ryan. Suggesting commit 15454.diff​ and close as fixed for 3.1, and we can revisit textarea_escaped instances in a new ticket. Alternatively, punt to 3.2-early, but a note on the attachment, esc_html() handles everything that esc_textarea() does except that it does not re-escape, and it does not handle &. So it should be considered safe. (And is far less destructive.)

[ryan]

(In [17146]) link_notes and term_description escaping fixes. Props garyc40. fixes #15454

[Denis-de-Bernardy]

@nacin or @ryan or @markjaquith: Any odds you could chime in when you've a moment, to explain when and why esc_textarea() is supposed to be used rather than esc_html()? The codex is rather unclear on when to use it, and this ticket and others related to it give the impression that the only thing we introduced with esc_textarea() were bugs and regressions.

The main difference I'm seeing between the two are:

• esc_textarea() will blissfully spit out invalid utf8 chars, whereas esc_html() will strip them
• esc_textarea() will double-encode html entities, whereas esc_html() will not

[nacin]

esc_textarea() will double-encode html entities, whereas esc_html() will not

That's the key difference.