Fix sanitization issue #15
Assignees
Comments
|
What we could do is divvy up these between Seth, Tony, and myself to clean them up. How does that sound Seth? |
|
I like that idea. How should we break it down? |
|
How about we start with POST calls: $_POSTTony
Seth
Josh
|
|
I created a branch for this called: |
|
Next up: $_REQUESTTony
Seth
Josh
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
From WP.org regarding the latest updates:
You still have sanitization issues.
Please sanitize, escape, and validate your POST calls
When you include POST/GET/REQUEST/FILE calls in your plugin, it's important to sanitize, validate, and escape them. The goal here is to prevent a user from accidentally sending trash data through the system, as well as protecting them from potential security issues.
SANITIZE: Data that is input (either by a user or automatically) must be sanitized. This lessens the possibility of XSS vulnerabilities and MITM attacks where posted data is subverted.
VALIDATE: All data should be validated as much as possible. Even when you sanitize, remember that you don’t want someone putting in ‘dog’ when the only valid values are numbers.
ESCAPE: Data that is output must be escaped properly, so it can't hijack admin screens. There are many esc_*() functions you can use to make sure you don't show people the wrong data.
To help you with this, WordPress comes with a number of sanitization and escaping functions. You can read about those here:
Remember: You must use the MOST appropriate functions for the context. If you’re sanitizing email, use sanitize_email(), if you’re outputting HTML, use esc_html(), and so on.
Clean everything, check everything, escape everything, and never trust the users to always have input sane data.
Some examples from your plugin:
event-espresso-free/includes/shortcodes.php:555: $event_id = $_REQUEST['event_id']; //If the first two are not being used, then get the event id from the url
event-espresso-free/includes/shortcodes.php:737: $event_id = $_REQUEST['event_id']; //If the first two are not being used, then get the event id from the url
event-espresso-free/includes/event-management/insert_event.php:57: 'repeat_by' => $_POST['recurrence_repeat_by'],
event-espresso-free/includes/event-management/insert_event.php:58: 'recurrence_regis_date_increment' => $_POST['recurrence_regis_date_increment'],
event-espresso-free/includes/event-management/insert_event.php:59: 'recurrence_manual_dates' => $_POST['recurrence_manual_dates'],
event-espresso-free/includes/event-management/insert_event.php:60: 'recurrence_manual_end_dates' => $_POST['recurrence_manual_end_dates'],
Calling file locations poorly
The way your plugin is referencing other files is not going to work with all setups of WordPress.
When you hardcode in paths, or assume that everyone has WordPress in the root of their domain, you cause anyone using 'Giving WordPress it's own directory' (a VERY common setup) to break. In addition, WordPress allows users to change the name of wp-content, so you would break anyone who choses to do so.
Please review the following link and update your plugin accordingly. And don't worry about supporting WordPress 2.x or lower. We don't encourage it nor expect you to do so, so save yourself some time and energy.
Some examples from your plugin:
event-espresso-free/includes/event-management/csv_import.php:100: $csvfile = "../wp-content/uploads/events.csv";
event-espresso-free/espresso.php:234: define('WP_CONTENT_DIR', ABSPATH . 'wp-content');
Note: You don't need to define WP_CONTENT_DIR.
Don’t use esc_ functions to sanitize
When sanitizing data, it’s important to use sanitization functions, not escape functions. The two work together, but are not interchangable.
Functions like esc_attr() do NOT sanitize anything, and should never be used for that purpose.
The sole exception to this is URLs, which can use esc_url() or esc_url_raw() when being saved.
Please review this document for help finding the most appropriate sanitization functions: https://developer.wordpress.org/plugins/security/securing-input/
Some examples from your plugin:
event-espresso-free/includes/category-management/add_cat_to_db.php:5: $category_name = isset($_REQUEST['category_name']) && !empty($_REQUEST['category_name']) ? esc_html($_REQUEST['category_name']) : '';
The text was updated successfully, but these errors were encountered: