libreform / wp-libre-form

Easy native HTML5 forms for WordPress. Version 1.5 is unmaintained, but works without issue. 2.0 has been rewritten from the ground, and can be found at https://github.com/libreform/libreform
https://wordpress.org/plugins/wp-libre-form
GNU General Public License v3.0
67 stars 27 forks source link

Double escaped html #38

Closed jpeltoniemi closed 7 years ago

jpeltoniemi commented 7 years ago

HTML entites are escaped while saving and again when displaying the submission in wp_admin.

See inc/wp-ajax.php, lines 50 and 53

Shouldn't the data be saved as-is and the functions using the data take care of escaping if necessary?

anttiviljami commented 7 years ago

This is the WordPress way. :) Post content always gets sanitised both on the way in and out, although using different filters.

jpeltoniemi commented 7 years ago

Well, anyway the output is still double escaped. Easy fix would be to trust that the data is always sanitized and not re-escape it when printing.

When will I learn to not assume anything good about WordPress :D

anttiviljami commented 7 years ago

I don't see how that's a bad thing ? Data needs to be escaped on the way in and sanitised on the way out

jpeltoniemi commented 7 years ago

I'd argue actual sanitization (not escaping html entities but removing invalid data) is good, depending on what you're saving. However just escaping html doesn't do any good from the database's point of view.

Anyway, if it's how WP has it, no use arguing here. The point was that there is a bug somewhere that causes html entities to be double escaped.

I did some more tests and it seems that at least multiline values don't get unescaped. Here's some input and output:

This typed in a form textarea

<fuufuu>
'"
</fuufuu>`

becomes this in wp-admin submission view

&lt;fuufuu&gt;
&#039;&quot;
&lt;/fuufuu&gt;`

and this typed in form textarea

<fuufuu>'"</fuufuu>

stays the same in wp-admin submission view

<fuufuu>'"</fuufuu>
jpeltoniemi commented 7 years ago

Did some more digging. According to WP docs data don't need to be escaped(https://codex.wordpress.org/Function_Reference/update_post_meta#Character_Escaping)

Post meta values are passed through the stripslashes() function upon being stored, so you will need to be careful when passing in values (such as JSON) that might include \ escaped characters.

Do not store escaped values Consider the JSON value {"key":"value with \"escaped quotes\""}:

<?php
$escaped_json = '{"key":"value with \\"escaped quotes\\""}';
update_post_meta( $id, 'escaped_json', $escaped_json );
$broken = get_post_meta( $id, 'escaped_json', true );
/*
$broken, after passing through stripslashes() ends up unparsable:
{"key":"value with "escaped quotes""}
*/
?>

Here's wplf-ajax.php lines 48-55

// add submission data as meta values
foreach( $_POST as $key => $value ) {
  if( !is_array($value) ) {
    add_post_meta($post_id, $key, esc_html( $value ), true);
  }
  else {
    add_post_meta($post_id, $key, esc_html( json_encode( $value ) ), true);
  }
}

The first esc_html is not required at all, but probably should be wp_slash in case data is inserted to $_POST via filters(like I did). The second esc_html actually breaks stuff. Should be wp_slash