[wp-trac] [WordPress Trac] #26626: WP_Upgrader::unpack_package() can overflow path name length limits (patch attached)

[WordPress Trac]

#26626: WP_Upgrader::unpack_package() can overflow path name length limits (patch
attached)
-----------------------------+------------------------------
 Reporter:  DavidAnderson    |       Owner:
     Type:  defect (bug)     |      Status:  new
 Priority:  normal           |   Milestone:  Awaiting Review
Component:  Upgrade/Install  |     Version:  trunk
 Severity:  normal           |  Resolution:
 Keywords:                   |
-----------------------------+------------------------------
Description changed by SergeyBiryukov:

Old description:

> I had no idea that in 2013, there were still operating systems with
> pathname length limits of only 256 characters. But, apparently there are
> - I've had 3 reports of it in the last week (which came after I included
> an SDK in one of my plugins that had a lot of sub-directories).
>
> Two of the victims got hit when unpacking the plugin through a normal
> update. To prevent this in future, I restructured my plugin.
>
> The other victim was running Bitnami WAMP, with PHP 5.4.22 (Windows NT
> AFLAPTOP 6.1 build 7601 (Windows 7 Business Edition Service Pack 1)
> i586), and he was accessing WP_Upgrader::unpack_package() via a restore
> operation in UpdraftPlus Backup/Restore
> (http://wordpress.org/plugins/updraftplus), which uses this method to
> unpack zip files.
>
> See http://wordpress.org/support/topic/restore-fails-could-not-create-
> directory for more information on that. Basically, it boiled down to
> "WP_Upgrader::unpack_package() uses the basename of the zip file to
> create a directory in WP_CONTENT_DIR/upgrade/". So, if the zip file name
> is very long, then a meaningful amount of the potential 256-character
> limit can be lost.
>
> The guy running Bitnami lost 80 characters to reach his upgrade folder:
> C:\Program Files\BitNami WAMP Stack\apps\wordpresslucid\htdocs\wp-
> content/upgrade. That leaves 176 characters.
>
> The plugin he was unpacking needed 98 characters for its longest
> pathname:
> /plugins/<slug>/opencloud/symed/Symfony/Component/EventDispatcher/EventDispatcherInterface.php.
> That leaves 78 characters to spare. Should be plenty... except for
> WP_Upgrader::unpack_package() wanting to use basename($zipfile) to create
> a directory for unpacking into. The zipfile did indeed have a filename of
> over 78 characters long. Boom!
>
> To prevent other scenarios in which someone might get hit by this, we can
> just change this line:
>
> $working_dir = $upgrade_folder . basename($package, '.zip');
>
> to:
>
> $working_dir = $upgrade_folder . substr(md5(basename($package, '.zip')),
> 0, 8);
>
> The use of md5() to prevent collisions might be over-cautious (especially
> given that WP_Upgrader::unpack_package() already tries to empty out the
> upgrades directory), but it's also harmless.

New description:

 I had no idea that in 2013, there were still operating systems with
 pathname length limits of only 256 characters. But, apparently there are -
 I've had 3 reports of it in the last week (which came after I included an
 SDK in one of my plugins that had a lot of sub-directories).

 Two of the victims got hit when unpacking the plugin through a normal
 update. To prevent this in future, I restructured my plugin.

 The other victim was running Bitnami WAMP, with PHP 5.4.22 (Windows NT
 AFLAPTOP 6.1 build 7601 (Windows 7 Business Edition Service Pack 1) i586),
 and he was accessing WP_Upgrader::unpack_package() via a restore operation
 in UpdraftPlus Backup/Restore (http://wordpress.org/plugins/updraftplus),
 which uses this method to unpack zip files.

 See http://wordpress.org/support/topic/restore-fails-could-not-create-
 directory for more information on that. Basically, it boiled down to
 "WP_Upgrader::unpack_package() uses the basename of the zip file to create
 a directory in WP_CONTENT_DIR/upgrade/". So, if the zip file name is very
 long, then a meaningful amount of the potential 256-character limit can be
 lost.

 The guy running Bitnami lost 80 characters to reach his upgrade folder:
 `C:\Program Files\BitNami WAMP Stack\apps\wordpresslucid\htdocs\wp-
 content/upgrade`. That leaves 176 characters.

 The plugin he was unpacking needed 98 characters for its longest pathname:
 `/plugins/<slug>/opencloud/symed/Symfony/Component/EventDispatcher/EventDispatcherInterface.php`.
 That leaves 78 characters to spare. Should be plenty... except for
 WP_Upgrader::unpack_package() wanting to use basename($zipfile) to create
 a directory for unpacking into. The zipfile did indeed have a filename of
 over 78 characters long. Boom!

 To prevent other scenarios in which someone might get hit by this, we can
 just change this line:
 {{{
 $working_dir = $upgrade_folder . basename($package, '.zip');
 }}}
 to:
 {{{
 $working_dir = $upgrade_folder . substr(md5(basename($package, '.zip')),
 0, 8);
 }}}
 The use of md5() to prevent collisions might be over-cautious (especially
 given that WP_Upgrader::unpack_package() already tries to empty out the
 upgrades directory), but it's also harmless.

--

--
Ticket URL: <http://core.trac.wordpress.org/ticket/26626#comment:1>
WordPress Trac <http://core.trac.wordpress.org/>
WordPress blogging software