Make WordPress Core

Opened 7 years ago

Last modified 21 months ago

#26626 new defect (bug)

WP_Upgrader::unpack_package() can overflow path name length limits (patch attached) — at Version 1

Reported by: DavidAnderson Owned by:
Milestone: Priority: normal
Severity: normal Version: 2.7
Component: Upgrade/Install Keywords: has-patch dev-feedback
Focuses: Cc:

Description (last modified by SergeyBiryukov)

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');


$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.

Change History (2)

7 years ago

1-line patch

#1 @SergeyBiryukov
7 years ago

  • Description modified (diff)
Note: See TracTickets for help on using tickets.