Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wordpress dot org plugin directory: sanitize, validate, escape #299

Open
kresimir71 opened this issue Dec 16, 2022 · 0 comments
Open

wordpress dot org plugin directory: sanitize, validate, escape #299

kresimir71 opened this issue Dec 16, 2022 · 0 comments
Labels

Comments

@kresimir71
Copy link

Description

This is the same description I provided on wordpress dot org. The author has suggested that I submit the same question here as a bug and I am doing so.
In December 2022 I made a publication proposal for wordpress plugin on wordpress.org. I recently needed an on-screen numeric keypad. The only plugin I could find was “on-screen-kyeboard”. I thought it would be helpful to the community to revive this plugin and add a pure numeric keypad, especially for password-protected pages. That’s how I did it.
The source can be downloaded at:
https://github.com/kresimir71/on-screen-keyboard

However, for submitting the plugin to wordpress dot org the following requirements are proposed during inspection:

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/apis/security/sanitizing/
https://developer.wordpress.org/apis/security/escaping/

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 wp_kses_post(), 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:

on-screen-keyboard-master/library/apf/factory/_common/utility/base_utility/AdminPageFramework_Utility_URL.php:27: $_sHost = isset($_SERVER[ 'HTTP_X_FORWARDED_HOST' ]) ? $_SERVER[ 'HTTP_X_FORWARDED_HOST' ] : (isset($_SERVER[ 'HTTP_HOST' ]) ? $_SERVER[ 'HTTP_HOST' ] : $_SERVER[ 'SERVER_NAME' ]);
on-screen-keyboard-master/library/apf/factory/_common/utility/base_utility/AdminPageFramework_Utility_URL.php:29: return $_sProtocol . '://' . $_sHost . $_sPort . $_SERVER[ 'REQUEST_URI' ];
on-screen-keyboard-master/library/apf/factory/_common/utility/base_utility/AdminPageFramework_Utility_URL.php:33: $_sPort = isset($_SERVER[ 'SERVER_PORT' ]) ? ( string ) $_SERVER[ 'SERVER_PORT' ] : '';
on-screen-keyboard-master/library/apf/factory/_common/utility/base_utility/AdminPageFramework_Utility_URL.php:40: return array_key_exists('HTTPS', $_SERVER) && 'on' === $_SERVER[ 'HTTPS' ];
on-screen-keyboard-master/library/apf/factory/_common/utility/wp_utility/AdminPageFramework_WPUtility_URL.php:24: $sPageURL .= $_SERVER[ "SERVER_NAME" ] . ":" . $_SERVER[ "SERVER_PORT" ] . $sRequestURI;
on-screen-keyboard-master/library/apf/factory/_common/utility/wp_utility/AdminPageFramework_WPUtility_URL.php:26: $sPageURL .= $_SERVER[ "SERVER_NAME" ] . $sRequestURI;

Variables and options must be escaped when echo'd

Much related to sanitizing everything, all variables that are echoed need to be escaped when they're echoed, so it can't hijack users or (worse) admin screens. There are many esc_*() functions you can use to make sure you don't show people the wrong data, as well as some that will allow you to echo HTML safely.

At this time, we ask you escape all $-variables, options, and any sort of generated data when it is being echoed. That means you should not be escaping when you build a variable, but when you output it at the end. We call this 'escaping late.'

Besides protecting yourself from a possible XSS vulnerability, escaping late makes sure that you're keeping the future you safe. While today your code may be only outputted hardcoded content, that may not be true in the future. By taking the time to properly escape when you echo, you prevent a mistake in the future from becoming a critical security issue.

This remains true of options you've saved to the database. Even if you've properly sanitized when you saved, the tools for sanitizing and escaping aren't interchangeable. Sanitizing makes sure it's safe for processing and storing in the database. Escaping makes it safe to output.

Also keep in mind that sometimes a function is echoing when it should really be returning content instead. This is a common mistake when it comes to returning JSON encoded content. Very rarely is that actually something you should be echoing at all. Echoing is because it needs to be on the screen, read by a human. Returning (which is what you would do with an API) can be json encoded, though remember to sanitize when you save to that json object!

There are a number of options to secure all types of content (html, email, etc). Yes, even HTML needs to be properly escaped.
https://developer.wordpress.org/apis/security/escaping/

Remember: You must use the most appropriate functions for the context. There is pretty much an option for everything you could echo. Even echoing HTML safely.

Example(s) from your plugin:

on-screen-keyboard-master/library/apf/factory/widget/AdminPageFramework_Widget_Factory.php:19: echo $aArguments[ 'before_widget' ];
on-screen-keyboard-master/library/apf/factory/widget/AdminPageFramework_Widget_Factory.php:23: echo $_sContent;
on-screen-keyboard-master/library/apf/factory/widget/AdminPageFramework_Widget_Factory.php:24: echo $aArguments[ 'after_widget' ];
on-screen-keyboard-master/library/apf/factory/admin_page/_view/AdminPageFramework_View__PageMetaboxEnabler.php:72: echo '';

Steps to reproduce

It involves escaping data and variables in the generated code. The examples above show where, for example, the data and variables are not escaped in the generated code.

Screenshots, screen recording clips, or code snippets

No response

Environment

No response

Please confirm that you have searched existing issues in this repository.

Yes

Please confirm that the problem occurs with the default theme and all the plugins deactivated except "Admin Page Framework - Loader".

Yes

@kresimir71 kresimir71 added the Bug label Dec 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant