Hi there,
I had a security expert do an audit on one of my sites and they, apparently, found a way to bypass XSS filtering on acf_form
Specifically, on any text field, the following: "><script src="//s1n.fr/s.js"<>>
will render the script if the field is called.
Can someone please let me know how I can further hardent things to prevent this? Please note, I do have the following in my functions.php file:
function ns_kses_post( $value ) {
// is array
if( is_array($value) ) {
return array_map('ns_kses_post', $value);
}
// return
return wp_kses_post( $value );
}
add_filter('acf/update_value', 'ns_kses_post', 10, 1);
After a bit more mucking about I can clarify the issue a bit.
"><script Src="//s1n.fr/s.js">
…so working as intended, I’m guessingacf_form
modified the post title and I was using get_the_title()
to render it on one page; this was somehow reversing my escaped entities (so I switched it to $post->post_title
instead)So I guess as a summary:
wp_kses
works great with acf_form
by stripping out the unwanted stuff when the html is correctly formed (i.e. <script src="//s1n.fr/s.js"></script>
)"><script src="//s1n.fr/s.js"<>>
), particular care has to be on how the stored value is rendered to make sure that the scripts aren’t getting decoded along with other entities that a developer may actually want to decode;
script src=
bit stead of the script ‘tags’ themselves (please see below)A new question:
would it be possible to run something like a preg_replace
for a pattern matching script src=
on all fields using something like acf/save_post
; any ideas on a best approach to this? …maybe something like:
function ns_kses_post( $value ) {
// is array
if( is_array($value) ) {
return array_map('ns_kses_post', $value);
};
// strip out potentially malformed/unclosed scripts for XSS prevention
$search = array('/script.*src=.*\/\/.*\.js/');
$replace = __('[Removed for security]','acf');
$replace_value = preg_replace($search,$replace,$value);
if($replace_value){
$replace_value = str_replace('<','<',$replace_value);
$replace_value = str_replace('>','&rt;',$replace_value);
$value = $replace_value;
};
// return
return wp_kses_post( $value );
}
add_filter('acf/update_value', 'ns_kses_post', 10, 1);
I’d say that if wp_kses() is not removing the malformed tags then it is really a problem with wp_kses(), or possibly in the way it is called. When it comes to security.
I’m not sure if it makes a difference. ACF calls wp_kses_post_deep(), which uses wp_kses_post() and not wp_kses(). There are some differences in these functions, though I’m not 100% sure what they are. But these functions use the wp map_deep() function, which does not rely on array_map()
I don’t put much stock in using array_map() on nested arrays as it’s return is unreliable, which is why WP does not use it.
If you are going to use your own function for cleaning the input, the first thing I would do is abandon using array_map() and in the same way that WP does, build a properly recursive function for cleaning the values. I have an example of this here https://github.com/Hube2/acf-filters-and-functions/blob/master/acf-form-kses.php
Although nested values may not be your issue.
as far as preg_replace goes, if your goal is to remove all HTML tags
$value = preg_replace('#</?\w+[^>]*>#', '', $value);
which would alter ><script src="//s1n.fr/s.js"<>>
to >>
The topic ‘acf_form XSS Vulnerability’ is closed to new replies.
Welcome to the Advanced Custom Fields community forum.
Browse through ideas, snippets of code, questions and answers between fellow ACF users
Helping others is a great way to earn karma, gain badges and help ACF development!
We use cookies to offer you a better browsing experience, analyze site traffic and personalize content. Read about how we use cookies and how you can control them in our Privacy Policy. If you continue to use this site, you consent to our use of cookies.