php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Doc Bug #72608 Example on manual page includes a directory traversal attack vulnerability
Submitted: 2016-07-17 15:25 UTC Modified: 2016-07-18 12:39 UTC
From: adrian dot testaavila at gmail dot com Assigned: cmb (profile)
Status: Duplicate Package: Documentation problem
PHP Version: Irrelevant OS: n/a
Private report: No CVE-ID: None
 [2016-07-17 15:25 UTC] adrian dot testaavila at gmail dot com
Description:
------------
---
From manual page: http://www.php.net/function.move-uploaded-file
---

Example #1 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

Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2016-07-18 12:39 UTC] cmb@php.net
-Status: Open +Status: Duplicate -Type: Security +Type: Documentation Problem -Assigned To: +Assigned To: cmb
 [2016-07-18 12:39 UTC] cmb@php.net
Duplicate of bug #72609.
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Tue Apr 16 17:01:30 2024 UTC