[wp-trac] [WordPress Trac] #44532: Extreme memory leak related to wp_is_stream in wp-includes/functions.php in WordPress 4.9.7

WordPress Trac noreply at wordpress.org
Tue Jul 17 04:17:17 UTC 2018


#44532: Extreme memory leak related to wp_is_stream in wp-includes/functions.php in
WordPress 4.9.7
------------------------------------------------+---------------------
 Reporter:  timbowesohft                        |       Owner:  (none)
     Type:  defect (bug)                        |      Status:  new
 Priority:  high                                |   Milestone:  4.9.8
Component:  Media                               |     Version:  4.9.7
 Severity:  major                               |  Resolution:
 Keywords:  has-patch reporter-feedback commit  |     Focuses:
------------------------------------------------+---------------------

Comment (by dontstealmyfish):

 Replying to [comment:46 pento]:
 > Thanks for the suggestion, @itowhid06. In this case, we don't need to
 add a special check. `file://` is still a stream, it's just a local
 stream, same as `rar://`, for example. if a path starting with `file://`
 is sent to `wp_is_stream()`, it should return `true`.
 >
 > The discussion in #43054 makes it pretty clear that this is a PHP memory
 corruption issue: a non-string value is somehow being returned by
 `stream_get_wrappers()`, which has two possible effects:
 >
 > - The random data is small when interpreted as a string. (ie, reading it
 as string quickly encounters a `NULL` byte, indicating the end of the
 string.) This was fixed in #43054.
 > - The random data is huge when interpreted as a string. This is the
 issue that we're seeing here.
 >
 > There are two ways of tackling this second issue:
 >
 > - As in [attachment:"44532.1.diff"], bailing if the path isn't a stream.
 This isn't perfect: it's still possible to trigger the PHP bug, but it
 does significantly reduce the chances of the bug being encountered. For
 many WordPress sites (those that don't have plugins that use streams), it
 reduces the chance to zero.
 >
 > - Parse each wrapper name. As @dd32 [ticket:43054#comment:5 mentioned on
 #43054], there are strict rules about what characters a wrapper name can
 contain. If we see an invalid character, bail on that wrapper name, and
 move onto the next one: it's a safe bet that a pointer to random memory
 will contain characters that aren't valid in a wrapper name: this would
 solve the issue for approximately 100% of cases. (There's a non-zero
 chance the random memory chunk we're given could actually be interpreted
 as data, and that the path sent to `wp_is_stream()` happens to match that
 random memory. 😉)
 >
 > I'm strongly inclined to go with option one. Option two is quite
 complex, its a performance drag for the vast majority of calls, and it'll
 hopefully be made unnecessary in a future PHP release. Option one is
 simple, understandable, and it provides a performance improvement!
 >
 > Unless any of the folks testing [attachment:"44532.1.diff"] are still
 seeing the problem, I'm fine with it being committed and backported to the
 4.9 branch in time for 4.9.8 beta 1.

 I have had no problems for days after implementing 44532.1 diff

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/44532#comment:48>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform


More information about the wp-trac mailing list