WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 3 years ago

Last modified 3 years ago

#30258 closed defect (bug) (duplicate)

Infinite loop in wp_mkdir_p in combination with open_basedir

Reported by: sowmedia Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.7
Component: Filesystem API Keywords: has-patch
Focuses: Cc:
PR Number:

Description

Option "upload_path" is set while hosting at ISP A. After switching to ISP B, where this path is outside of the open_basedir, the site enters a loop at wp-includes/functions.php:1497 that is only exitted when the maximum execution time of php is reached.

I would suggest:

while ( '.' != $target_parent && ! is_dir( $target_parent ) ) {
  if(dirname($target_parent)==$target_parent)
  return false; // Root reached
  $target_parent = dirname( $target_parent );
}

Attachments (2)

fix-for-ticket-30258-using-basedir.patch (592 bytes) - added by JxsDotNL 4 years ago.
Proposed fix for ticket 30258 - as suggested above
fix-for-ticket-30258-cpu-efficient.patch (815 bytes) - added by JxsDotNL 4 years ago.
Proposed fix for ticket 30258 - using temp vars, just a little more CPU efficient

Download all attachments as: .zip

Change History (15)

#1 @jeremyfelt
5 years ago

  • Component changed from General to Filesystem API
  • Keywords reporter-feedback added
  • Version changed from trunk to 3.7

Hi @sowmedia, thanks for the report.

The current recursive directory handling was added via #23196.

I've attempted to reproduce what would cause the while() loop, but dirname() continues to work for me as expected when checking directories outside of a configured open_basedir setting. Can you provide an example of the config you're dealing with?

#2 @sowmedia
5 years ago

For example, we take a website example.com hosted at provider X, where it is located in /home/example/public_html.
We set the upload dir with set_option('upload_path',ABSPATH.'/wp-content/uploads');

Then we do an export of the site using ftp/backwpup/backupbuddy or any other backup tool one may prefer, and import it at provider Y, where the site it located in /var/www/virtuals/example.com. The backend will not show before we do a delete_option('upload_path') or delete from wp_options where option_name='upload_path'; In our test, the frontend did work, the backend did not.

To reproduce this problem, we put the following code in /wp-content/mu-plugins/test.php and visited /wp-admin:

        $fake_uploadpath='/tmp/virtuals/itsnothere';
        $open_basedir=ini_get('open_basedir');
        if(!strlen($open_basedir)) die("open_basedir not set - bug won't show\n");
        foreach(preg_split("/:/",$open_basedir) as $r){
                if(preg_match(":^$r:",$fake_uploadpath))
                        die($fake_uploadpath." in open_basedir ($open_basedir) - bug won't show\n");
        }
        update_option('upload_path',$fake_uploadpath);
        error_reporting(E_ALL);

That results in thousands of errors and finally reaching the maximum execution time. You may need to visit /wp-admin twice if the mu-plugins are loaded after the buggy php code is run.

Finally, we remove the wrong setting using:

        update_option('upload_path','');

Your latest test was WP4.0.1

#3 @sowmedia
5 years ago

  • Keywords reporter-feedback removed

#4 @yr8u5
4 years ago

Dear all of Wordpress Develop Members.
Thank you for your continued support for us at all times.

I also made sure this issue in WP4.3.1.
In addition, @sowmedia's fix can avoid a issue.

[Making reproducible environment]

  • PHP ver5.5 on Linux-based server.
  • For example, set open_basedir in php.ini.
    ------------------------------
    open_basedir = "/home/example-user/example.com/www"
    ------------------------------
    * If you specify the current directory that have installed WordPress,[[BR]]
      it will be easier.
    
  • Log in to the dashboard,
    and access the "/wp-admin/options.php".

    By "upload_path" item,
    It will specify the outside of open_basedir area.
    ------------------------------
    /Home/example-user/images
    ------------------------------
    
  • This issue is create infinite loop.
    set to "max_execution_time" in php.ini
    It is recommended that you keep to the order of a few seconds,
    And "display_errors" in php.ini is safer to disable.



[confirm reproducible]

  • Dashboard in the "media> Add New",
    upload the image data.

    The following error is output in large amounts in the error log.
      ------------------------------
      PHP Warning: is_dir(): open_basedir restriction in effect.
      ------------------------------
      * If the error of upload_tmp_dir,
        Please change the upload_tmp_dir path to inside of the open_basedir area.
        ------------------------------
        upload_tmp_dir = "/home/example-user/example.com/www/images_tmp"
        ------------------------------
    



[fix to this issue]
Modifying to functions.php as proposed from @sowmedia
break to infinite loop and avoid to this issue.


Best regards,

#5 @JxsDotNL
4 years ago

  • Version 3.7 deleted

Hi all,

(I originally discovered this issue when working on a project for @Sowmedia and asked them to report.)

This issue is still present in the current the 4.4 beta available at the SVN trunk right now. I'll be proposing a patch in a few minutes.

I'll be submitting two options:

  • fix-for-ticket-30258-using-basedir.patch - which is the fix I originally suggested
  • fix-for-ticket-30258-cpu-efficient.patch - which uses a temp variable instead of the basedir function. I believe this is just a little more cpu efficient.

@JxsDotNL
4 years ago

Proposed fix for ticket 30258 - as suggested above

@JxsDotNL
4 years ago

Proposed fix for ticket 30258 - using temp vars, just a little more CPU efficient

#6 @JxsDotNL
4 years ago

  • Version set to trunk

#7 @JxsDotNL
4 years ago

  • Summary changed from Upload_path can cause a loop after migrating to Infinite loop in wp_mkdir_p in combination with open_basedir

#8 @JxsDotNL
4 years ago

#30642 was marked as a duplicate.

#9 @jeremyfelt
4 years ago

  • Keywords has-patch added
  • Version changed from trunk to 3.7

Version is used to track when a bug is introduced, in this case it appears to be 3.7.

#10 @JxsDotNL
4 years ago

It's been 19 months since this thread was opened, and 7 months since I have submitted two possible solutions. I see the bug is still present in the most recent release of WP.
Is there anything else I need to do to get this bug fixed? It's just a small bug, but I've ran into it several times.
Thanks!

#11 @robertstaddon
3 years ago

A duplicate ticket with the same problem is here: https://core.trac.wordpress.org/ticket/36184

I just submitted a patch on the above ticket that seems to be a little cleaner solution than this one. @JxsDotNL and @yr8u5 and @sowmedia , would any of you like to test it?

#12 @robertstaddon
3 years ago

  • Resolution set to duplicate
  • Status changed from new to closed

Duplicate of #36184.

#13 @swissspidy
3 years ago

  • Milestone Awaiting Review deleted
Note: See TracTickets for help on using tickets.