skaut / levitio-connection

Podpůrný plugin Levitio pro WordPress. Nyní podporuje zobrazování výprav na webu.
https://wordpress.org/plugins/levitio-connection/
GNU General Public License v3.0
2 stars 0 forks source link

Připomínky z wp.org #2

Closed kalich5 closed 3 years ago

kalich5 commented 4 years ago

There are issues with your plugin code preventing it from being approved immediately. We have pended your submission in order to help you correct all issues so that it may be approved and published.

We ask you read this email in its entirety, address all listed issues, and reply to this email with your corrected code attached (or linked). You have 6 months to make all corrections, before your plugin will be rejected. Even so, as long as you reply to this email, we will be able to continue with your review and eventually publish your code.

Remember in addition to code quality, security and functionality, we require all plugins adhere to our guidelines. If you have not yet, please read them:

https://developer.wordpress.org/plugins/wordpress-org/detailed-plugin-guidelines/

We know it can be long, but you must follow the directions at the end as not doing so will result in your review being delayed. It is required for you to read and reply to these emails, and failure to do so will result in significant delays with your plugin being accepted.

Be aware that you will not be able to submit another plugin while this one is being reviewed.

Data Must be Sanitized, Escaped, and Validated

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 as soon as possible. This lessens the possibility of XSS vulnerabilities and MITM attacks where posted data is subverted.

VALIDATE: All data should be validated, no matter what. 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 when it is echo'd, 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:

https://developer.wordpress.org/plugins/security/securing-input/ https://developer.wordpress.org/plugins/security/securing-output/

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.

An easy mantra here is this:

Sanitize early Escape Late Always Validate

Clean everything, check everything, escape everything, and never trust the users to always have input sane data. After all, users come from all walks of life.

Example(s) from your plugin:

skautappka-connection/skautappka-connection.php:380: $settings[ $option['id'] ] = $_REQUEST[ $option['id'] ]; skautappka-connection/skautappka-options-page.php:13:

<?php echo $errors[$_GET['error']]; ?>

Allowing Direct File Access to plugin files

Direct file access is when someone directly queries your file. This can be done by simply entering the complete path to the file in the URL bar of the browser but can also be done by doing a POST request directly to the file. For files that only contain a PHP class the risk of something funky happening when directly accessed is pretty small. For files that contain procedural code, functions and function calls, the chance of security risks is a lot bigger.

You can avoid this by putting this code at the top of all php files:

if ( ! defined( 'ABSPATH' ) ) exit; // Exit if accessed directly


We believe this to be a complete review of all issues found in your plugin. If we have no response from this email address in 6 months, we will reject this submission in order to keep our queue manageable. To keep your review active, all we ask is that you make corrections and reply.

Your next steps are:

Make all the corrections related to the issues we listed. Review your entire code to ensure there are no other related concerns. Attach your corrected plugin as a zip file OR provide a link to a public location (Dropbox, Github, etc) from where we can download the code.

Once we receive your updated code, we will re-review it from top down.

Be aware that if your zip contains javascript files, you may not be able to email it as many hosts block that in the interests of security. Also note that all version control directories (like Github) will auto-generate a zip for you.

While we have tried to make this review as exhaustive as possible we, like you, are humans and may have missed things. As such, we will re-review the entire plugin when you send it back to us. We appreciate your patience and understanding.

If you have questions, concerns, or need clarification, please reply to this email and just ask us.

kalich5 commented 4 years ago

There are still issues.

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 like wp-content or your plugin folder name, 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 chooses 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.

Remember to make use of the FILE variable, in order than your plugin function properly in the real world.

Example(s) from your plugin:

skautappka-connection-1.0/SkautAppkaWidget.php:318: . file_get_contents(WP_PLUGIN_DIR.'/skautappka-connection/updateVyprava.js') skautappka-connection-1.0/SkautAppkaWidget.php:500: $outputHtml .= file_get_contents(WP_PLUGIN_DIR.'/skautappka-connection/options-page.html');

Data Must be Sanitized, Escaped, and Validated

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 as soon as possible. This lessens the possibility of XSS vulnerabilities and MITM attacks where posted data is subverted.

VALIDATE: All data should be validated, no matter what. 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 when it is echo'd, 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:

https://developer.wordpress.org/plugins/security/securing-input/ https://developer.wordpress.org/plugins/security/securing-output/

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.

An easy mantra here is this:

Sanitize early Escape Late Always Validate

Clean everything, check everything, escape everything, and never trust the users to always have input sane data. After all, users come from all walks of life.

Example(s) from your plugin:

skautappka-connection-1.0/SkautAppkaWidget.php:453: $outputHtml .= '

'.$messages[$_GET['message']].'

';

Undocumented use of a 3rd Party or external service

We permit plugins to require the use of 3rd party (i.e. external) services, provided they are properly documented in a clear manner.

We require plugins that reach out to other services to disclose this, in clear and plain language, so users are aware of where data is being sent. This allows them to ensure that any legal issues with data transmissions are covered. This is true even if you are the 3rd party service.

In order to do so, you must update your readme to do the following:

clearly explain that your plugin is relying on a 3rd party as a service and under what circumstances provide a link to the service provide a link to the services’ a terms of use and/or privacy policies

Remember, this is for your own legal protection. Use of services must be upfront and well documented.

Example(s) from your plugin:

https://www.skautappka.cz/

kalich5 commented 4 years ago

include_once('skautappka-util.php'); // Utility functions for backward compatibility

That;s just calling wp_doing_ajax() if it doesn't exist.

This should not be needed at all. What errors were you facing that required that?

kalich5 commented 3 years ago

vyřešeno a schváleno