search

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

Sanitize all input? #42

Closed ottok closed 5 years ago

ottok commented 7 years ago

I am not sure if I read the code correctly (and project code layout description at the end of the README.md could help) but it looks like the data from the form is taken from _POST and stored into database here without using the WordPress sanitize functions: https://github.com/anttiviljami/wp-libre-form/blob/master/inc/wplf-ajax.php#L47-L55

Only html escaping is used, but that is the thing you should do when printing the data, not when storing it.

The post_title is however saved correctly with sanitize_textfield().

Once 4.7 is released and it contains support for improved sanitize functions I could extend the POST data looper and add proper validation functions to each element in the array, but I also still need to figure out how to track what the HTML5 form datatype is to correctly validate/sanitize per datatype.

anttiviljami commented 7 years ago

This seems related to #40 and #38

anttiviljami commented 7 years ago

PR #40 seems to make it possible to add your own sanitisation via wplf_form_data filter, which at the moment isn't supported without modifying the global $_POST array, which is kind of ugly. I like the idea of giving the developer the ability to easily add sanitisation.

Not too sure if we should force data sanitisation based on input data types.

@jpeltoniemi thoughts ?

jpeltoniemi commented 7 years ago

I don't mean to hijack this project, but wplf-ajax.php has had a pretty much complete overhaul. I've been careful not to break anything but the mechanics are somewhat different. So any issues regarding wplf-ajax.php are probably now non-issues, if my changes are satisfactory.

@ottok I'm no expert on WP internals but I'm fairly sure there are safeguards against SQL injections in WP itself(if that's what you were after). As I said above, wplf-ajax has changed pretty much and html escaping has been removed too. This is explained in #38 too.