WordPress.org

Make WordPress Core

Opened 4 months ago

Closed 3 months ago

#43417 closed defect (bug) (fixed)

Infinite loop in wp_mkdir_p with open_basedir restrictions

Reported by: 1265578519 Owned by: SergeyBiryukov
Milestone: 4.9.5 Priority: normal
Severity: normal Version: 3.7
Component: Filesystem API Keywords: has-patch fixed-major
Focuses: Cc:

Description

https://i.imgur.com/OBYbXT6.png Cause the head exceeds the 4M limit.

/wwwroot/wp-includes/functions.php 1618 https://i.imgur.com/KniAKB5.png modify is_dir front @ cpu 100%.

Attachments (1)

43417.diff (626 bytes) - added by soulseekah 4 months ago.
Initial fix attempt

Download all attachments as: .zip

Change History (10)

#1 @soulseekah
4 months ago

  • Component changed from Canonical to Filesystem API
  • Summary changed from upstream protocol header size is too big to Infinite loop in wp_mkdir_p with open_basedir restrictions

It's an infinite loop :D to reproduce:

<?php
ini_set( 'open_basedir', '/tmp/' );
wp_mkdir_p( '/hello/world/' );
Last edited 4 months ago by soulseekah (previous) (diff)

@soulseekah
4 months ago

Initial fix attempt

#2 @soulseekah
4 months ago

  • Keywords has-patch needs-unit-tests added

The test is tough to write, once ini_set is called the whole test suite breaks. Plus the test can't fail, because it's in an infinite loop, it can only pass.

<?php
        /**
         * @ticket 43417
         */
        public function test_wp_mkdir_p_loop() {
                ini_set( 'open_basedir', '/no/exists/ever' );

                // suppress open_basedir restriction errors
                @wp_mkdir_p( '/tmp/try/his' );

                ini_restore( 'open_basedir' );
        }

#3 @1265578519
4 months ago

Thanks, 43417.diff successfully solved the problem .

#4 @SergeyBiryukov
4 months ago

  • Milestone changed from Awaiting Review to 5.0

#5 @soulseekah
3 months ago

Didn't think I'd ever encounter this, but I just saw this happen on a migrated client site via a call to wp_upload_dir from Divi theme, where the et_images_temp_folder option was not replaced correctly.

An incorrect/badly formed upload_path would also get into an infinite loop. So this is not a theoretical scenario, this actually happens :(

Patch got rid of infinite loop in wp_mkdir_p call.

Any ideas on how to automate test for this? Or can this be merged without unit tests due to it being so dangerous to test?

#6 @SergeyBiryukov
3 months ago

  • Version changed from trunk to 3.7

Introduced in [25047].

#7 @SergeyBiryukov
3 months ago

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

In 42801:

Filesystem API: Avoid an infinite loop in wp_mkdir_p() when trying to determine the parent folder with open_basedir restriction in effect.

Props soulseekah, 1265578519-1.
Fixes #43417.

#8 @SergeyBiryukov
3 months ago

  • Keywords fixed-major added; needs-unit-tests removed
  • Milestone changed from 5.0 to 4.9.5
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for 4.9.5 consideration.

#9 @SergeyBiryukov
3 months ago

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

In 42804:

Filesystem API: Avoid an infinite loop in wp_mkdir_p() when trying to determine the parent folder with open_basedir restriction in effect.

Props soulseekah, 1265578519-1.
Merges [42801] to the 4.9 branch.
Fixes #43417.

Note: See TracTickets for help on using tickets.