Make WordPress Core

Opened 7 years ago

Closed 7 years ago

#43054 closed defect (bug) (fixed)

wp_is_stream fails with stream definition containing nonascii chars

Reported by: tpaksu's profile tpaksu Owned by: dd32's profile dd32
Milestone: 4.9.8 Priority: normal
Severity: normal Version: 4.9.1
Component: Filesystem API Keywords: fixed-major
Focuses: Cc:

Description

Hi,

My stream_get_wrappers function returns this array:

array(11) {
  [0]=>
  string(3) "php"
  [1]=>
  string(4) "file"
  [2]=>
  string(4) "glob"
  [3]=>
  string(4) "data"
  [4]=>
  string(0) ""
  [5]=>
  string(163) "0���>�s>����1�DNS_A�>�s>������>�s>��h0�R�DNS_NS�>�"
  [6]=>
  string(3) "zip"
  [7]=>
  string(13) "compress.zlib"
  [8]=>
  string(5) "https"
  [9]=>
  string(4) "ftps"
  [10]=>
  string(4) "phar"
}

I don't know what's causing the 5th indexed element to be displayed, but it breaks my wordpress page with this error:

Warning:  preg_match(): Null byte in regex in /wp-includes/functions.php on line 5220

Note: Line number may be different because I added some debugging codes.

And I'm using Smart Slider 3 which obviously calls wp_upload_dir() which calls wp_mkdir_p() and it calls wp_is_stream() which eventually gives this warning.

I suppose this function may use a check like this:

<?php
$wrappers = stream_get_wrappers();
$wrappers = array_filter($wrappers, function($wrapper){
    return preg_match("!^\w+$!", $wrapper);
});

Change History (10)

#1 @subrataemfluence
7 years ago

  • Keywords reporter-feedback added

Hi,
Though I am not sure but can you please try by deactivating Smart Slider 3 plugin.
It seems like the this plugin has some effect!

#2 follow-up: @tpaksu
7 years ago

I now can't reproduce it because I changed the settings of the plugin and applied that fix that I've mentioned in the issue. Maybe some setting was interfering with the streams in that plugin which caused another stream to be added to the array. But when the issue was prominent, I tried disabling and uninstalling the plugin (I found out the error is caused by that plugin with debug_print_backtrace() method) but the warning was still there.

I removed that fix and now the warning doesn't show up. Interesting.

#3 in reply to: ↑ 2 @subrataemfluence
7 years ago

Replying to tpaksu:

So what you basically mean is the issue was caused due to a function in the plugin itself and it is not a WordPress core bug. Is this right?

I now can't reproduce it because I changed the settings of the plugin and applied that fix that I've mentioned in the issue. Maybe some setting was interfering with the streams in that plugin which caused another stream to be added to the array. But when the issue was prominent, I tried disabling and uninstalling the plugin (I found out the error is caused by that plugin with debug_print_backtrace() method) but the warning was still there.

I removed that fix and now the warning doesn't show up. Interesting.

#4 @tpaksu
7 years ago

Nope, I still don't know what was causing the nonascii output of stream_get_wrappers(). It may occur again with different plugins or themes because it's affecting the core methods wp_mkdir_p and wp_upload_dir(). I still think it as a failsafe solution.

#5 @dd32
7 years ago

  • Keywords reporter-feedback removed
  • Milestone changed from Awaiting Review to 5.0

This sounds like a PHP Memory corruption issue, as the stream wrapper name looks completely invalid.

For reference, PHP allows the following characters in a stream wrapper name: a-z, A-Z, 0-9, +, -, .. The value you've got in there looks like a raw DNS packet data - which certainly shouldn't be there. If this is your own server, update/restart PHP and ensure everything else is up to date.

However, that being said, this function still needs some more escaping, for example, if you have a compress.zlib wrapper installed (which most PHP's do) this will return true incorrectly.

var_dump( wp_is_stream( "compressZzlib://testing/invalid/stuff" ) );

Fixing this and avoiding the warning generated in your case here is the same change, so lets fix that.

#6 follow-up: @dd32
7 years ago

  • Owner set to dd32
  • Resolution set to fixed
  • Status changed from new to closed

In 42432:

Streams: When checking in wp_is_stream() escape the stream wrapper names for PCRE to avoid PHP warnings when invalid stream wrappers are registered.

Fixes #43054.

#7 in reply to: ↑ 6 @tpaksu
7 years ago

Replying to dd32:

In 42432:

Streams: When checking in wp_is_stream() escape the stream wrapper names for PCRE to avoid PHP warnings when invalid stream wrappers are registered.

Fixes #43054.

Yes, this is better. Thank you.

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


7 years ago

#9 @SergeyBiryukov
7 years ago

  • Keywords fixed-major added
  • Milestone changed from 5.0 to 4.9.8
  • Resolution fixed deleted
  • Status changed from closed to reopened

Moving for 4.9.8 consideration, along with #44532.

#10 @SergeyBiryukov
7 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 43483:

Streams: When checking in wp_is_stream() escape the stream wrapper names for PCRE to avoid PHP warnings when invalid stream wrappers are registered.

Props dd32.
Merges [42432] to the 4.9 branch.
Fixes #43054.

Note: See TracTickets for help on using tickets.