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

doesn't pass WordPress validation #41

Open
ziodave opened this issue Feb 1, 2022 · 0 comments
Open

doesn't pass WordPress validation #41

ziodave opened this issue Feb 1, 2022 · 0 comments

Comments

@ziodave
Copy link

ziodave commented Feb 1, 2022

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 (except for esc_url(), and yes, we know that's confusing). 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/plugins/security/securing-output/

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:

all-import-wordlift-add-on/rapid-addon.php:566: echo pmai_render_field($fieldData, ( ! empty($current_values) ) ? $current_values : array() );
all-import-wordlift-add-on/rapid-addon.php:570: <h4><?php _e($field_params['name'], 'wp_all_import_plugin'); <?php if ( ! empty($field_params['tooltip'])): <a href="#help" title="<?php echo $field_params['tooltip']; " style="position:relative; top: -1px;">?</a><?php endif; </h4>
all-import-wordlift-add-on/rapid-addon.php:575: echo $field_params['name'];
all-import-wordlift-add-on/rapid-addon.php:578: <p style="margin: 0 0 12px 0;"><?php echo $field_params['name'];</p>

You cannot translate $-variables or defines

It is impossible to use the WordPress translation system on dynamic code. A correct, ready to translate, example would be this:

__( 'This string needs to be translated.', 'my-plugin-permalink' )

By using 'my-plugin-permalink' and having it match the actual permalink of a plugin, you tell the system which plugin is translating and what it's translating.

However if you try to do this, it won't work:

__( $string_to_translate, 'my-plugin-permalink' )

The system doesn't know how to look back for $string_to_translate which means your plugin simply will not be translated properly.

Example(s) from your plugin:

_e($field_params['name'], 'wp_all_import_plugin');
cmdmcs pushed a commit that referenced this issue Feb 5, 2022
Programmer095 added a commit to Programmer095/wp-all-import-rapid-addon that referenced this issue Oct 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant