WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 3 months ago

#37698 reopened defect (bug)

wp_kses_split global variable pollution

Reported by: xknown Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: General Keywords: bulk-reopened
Focuses: Cc:

Description

In r10339, wp_kses_split was modified so it doesn't longer require the preg_replace with the e (eval) modifier. This implementation uses globals to pass the values of $allowed_html and $allowed_protocols to the _wp_kses_split_callback function.
While in most cases this isn't really a problem, we noticed that a call to wp_kses_split (via a filter) from within _wp_kses_split_callback may have undesirable effects on the next replacements.
The snippet below illustrates this problem, you can see in action in ​https://3v4l.org/YmYTZ

<?php

function wp_kses_split( $string, $allowed_html, $allowed_protocols ) {
    global $pass_allowed_html, $pass_allowed_protocols;
    $pass_allowed_html = $allowed_html;
    $pass_allowed_protocols = $allowed_protocols;
    return preg_replace_callback( '%((<!--.*?(-->|$))|(<[^>]*(>|$)|>))%', '_wp_kses_split_callback', $string );
}
function _wp_kses_split_callback( $match ) {
    global $pass_allowed_html, $pass_allowed_protocols;
    return wp_kses_split2( $match[1], $pass_allowed_html, $pass_allowed_protocols );
}

function wp_kses_split2($string, $allowed_html, $allowed_protocols) {
    wp_kses_split('', array(), array()); // this overrides the globals.
    print_r( array( $allowed_html, $allowed_protocols ) );
}

wp_kses_split("<a style='color: red;'>I link this</a>", array('a'=>array( 'style' => array() )), array('http') );

One way to fix this would be to use an anonymous function, but I guess that's only available on PHP >= 5.3. Another way is to encapsulate the callback in a class and tie the arguments to an instance of this class.

Change History (2)

#1 @iseulde
5 months ago

  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Status changed from new to closed

This ticket has not seen any activity in over *two* years, so I'm closing it as "wontfix".

The ticket may lack decisiveness, may have become irrelevant, or may not have gathered enough interest.

If you think this ticket does deserve some attention again, feel free to reopen.

For bugs, it would be great if you could provide updated steps to reproduce against the latest version of WordPress (5.0.2 at the time of writing). Remember images or a video can be superior to explain a problem. At the very least, quickly test again to make sure the bug still exists.

If it’s an enhancement or feature, some extra motivation may help.

Thank you for your contributions to WordPress! <3

#2 @JeffPaul
3 months ago

  • Keywords bulk-reopened added
  • Milestone set to Awaiting Review
  • Resolution wontfix deleted
  • Status changed from closed to reopened

A ​decision was made to reopen tickets that were closed in the bulk edit that this ticket was affected by. This ticket is being placed back into the Awaiting Review milestone so it can be individually evaluated and verified to determine if it is still relevant/valid or reproducible.

Note: See TracTickets for help on using tickets.