Doc #72609 [NEW]: Insecure example in php.net documentation

From: adrian dot testaavila at gmail dot com Operating system: n/a PHP version: Irrelevant Package: Website problem Bug Type: Documentation Problem Bug description:Insecure example in php.net documentation Description: ------------ --- From manual page: http://www.php.net/function.move-uploaded-file --- Example #1 (which is intended to be a _good_ example) demonstrates a filesystem traversal attack by allowing the $destination filename to be determined by user input: <?php $name = $_FILES["pictures"]["name"][$key]; move_uploaded_file($tmp_name, "$uploads_dir/$name"); ?> Examples in the manual should demonstrate best practices, especially where security is involved. Suggest changing the example to include validation for the $destination path, e.g., <?php $uploads_dir = '/uploads'; foreach ($_FILES["pictures"]["error"] as $key => $error) { if ($error == UPLOAD_ERR_OK) { $tmp_name = $_FILES["pictures"]["tmp_name"][$key]; $name = $_FILES["pictures"]["name"][$key]; // $name comes from user input, so we MUST NOT trust that it is safe or correct. // realpath() will fail if the destination directory does not exist, // and then we check that the destination directory *is* the uploads directory, as intended. $destination = "{$uploads_dir}/{$name}"; $dest_dir = realpath(dirname($destination)); if ($dest_dir !== $uploads_dir) { trigger_error('Bad file name!', E_USER_WARNING); } else { move_uploaded_file($tmp_name, $destination); } } } ?> Alternatively, the example could not use the user-provided filename at all; generating a hash or random string as the $destination filename. Also suggest adding a "Warning" box explaining the risks involved in allowing a user to choose where to save files on your filesystem. Test script: --------------- n/a Expected result: ---------------- n/a Actual result: -------------- n/a -- Edit bug report at https://bugs.php.net/bug.php?id=72609&edit=1 -- Try a snapshot (PHP 5.4): https://bugs.php.net/fix.php?id=72609&r=trysnapshot54 Try a snapshot (PHP 5.5): https://bugs.php.net/fix.php?id=72609&r=trysnapshot55 Try a snapshot (trunk): https://bugs.php.net/fix.php?id=72609&r=trysnapshottrunk Fixed in SVN: https://bugs.php.net/fix.php?id=72609&r=fixed Fixed in release: https://bugs.php.net/fix.php?id=72609&r=alreadyfixed Need backtrace: https://bugs.php.net/fix.php?id=72609&r=needtrace Need Reproduce Script: https://bugs.php.net/fix.php?id=72609&r=needscript Try newer version: https://bugs.php.net/fix.php?id=72609&r=oldversion Not developer issue: https://bugs.php.net/fix.php?id=72609&r=support Expected behavior: https://bugs.php.net/fix.php?id=72609&r=notwrong Not enough info: https://bugs.php.net/fix.php?id=72609&r=notenoughinfo Submitted twice: https://bugs.php.net/fix.php?id=72609&r=submittedtwice register_globals: https://bugs.php.net/fix.php?id=72609&r=globals PHP 4 support discontinued: https://bugs.php.net/fix.php?id=72609&r=php4 Daylight Savings: https://bugs.php.net/fix.php?id=72609&r=dst IIS Stability: https://bugs.php.net/fix.php?id=72609&r=isapi Install GNU Sed: https://bugs.php.net/fix.php?id=72609&r=gnused Floating point limitations: https://bugs.php.net/fix.php?id=72609&r=float No Zend Extensions: https://bugs.php.net/fix.php?id=72609&r=nozend MySQL Configuration Error: https://bugs.php.net/fix.php?id=72609&r=mysqlcfg