[wp-trac] [WordPress Trac] #48316: Changeset 46482 breaks upload when using ".." in upload_path.

[WordPress Trac]

#48316: Changeset 46482 breaks upload when using ".." in upload_path.
----------------------------+------------------------------
 Reporter:  xpoon           |       Owner:  (none)
     Type:  defect (bug)    |      Status:  reopened
 Priority:  normal          |   Milestone:  Awaiting Review
Component:  Filesystem API  |     Version:  trunk
 Severity:  major           |  Resolution:
 Keywords:                  |     Focuses:
----------------------------+------------------------------

Comment (by mpcube):

 What? No. Redefining ABSPATH? Would that even work? It's a constant!

 What i meant:

 Upload directory is constructed in function _wp_upload_dir(), where
 ABSPATH is concatenated with UPLOADS. For easier explanation let's look at
 an example:

 My Website is in

           /var/www/customername/www.example.com

 but as i have WordPress installed in a Subdirectory, ABSPATH will be

           /var/www/customername/www.example.com/wp/

 I'd like to have uploads in

           /var/www/customername/www.example.com/uploads/

 so I set UPLOADS to

           ../uploads/

 When _wp_upload_dir() is asked for the directory to put the uploads in, it
 simply concatenates ABSPATH with UPLOADS:

 {{{#!php
 $dir = ABSPATH . UPLOADS;
 }}}

 This results is the somehow not-so-nice upload_dir()

           /var/www/customername/www.example.com/wp/../uploads/

 This worked unil now, but now no directorys can be created any more
 beacuse the path has a '../' in it. Obviously at the point, where
 directories are made, the $path is not trusted for some reason, we don't
 want to go one level up on '../'.

 So we need to get rid of the '../' at a point where we know, that there's
 no harm in interpreting the string '../' as 'go up one level' - which is
 in _wp_upload_dir() where we know that this comes from constants in our
 code and not from some user input. At this point we can trust the values
 (both are constants) and strip of 'wp/../' (in my example).

 I think it would be better to modify _wp_upload_dir() in a way that
 upload_dir() becomes

           /var/www/customername/www.example.com/uploads/

 just by instead of

 {{{#!php
 $dir = ABSPATH . UPLOADS;
 }}}

 writing

 {{{#!php
         $uploads = UPLOADS;
         $base_parts = explode ( '/', trim(ABSPATH, '/' ) );
         while ( substr ( $uploads , 0, 3 ) == '../' ) {
                 $uploads = substr($uploads, 3);
                 array_pop($base_parts);
         }
         $dir = '/' . implode( '/', $base_parts );
         $dir .= '/' . ltrim($uploads, '/');
 }}}

 All that would be easier if i knew the reasons for the initial change
 46482 - but I can't find any information about that.

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