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
Welcome back! If you're the original bug submitter, here's where you can edit the bug or add additional notes.
If you forgot your password, you can retrieve your password here.
Password:
Status:
Package:
Bug Type:
Summary:
From: adrian dot testaavila at gmail dot com
New email:
PHP Version: OS:

 

 [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

Pull Requests

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: Sun Dec 22 12:01:30 2024 UTC