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, it doesn't seem to be worrying about the charset, and this ticket and others related to it give the impression that the only thing we introduced with esc_textarea() were bugs and regressions.

[nacin]

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

That's the key difference.