Sec Bug->Doc #72608 [Opn->Dup]: Example on manual page includes a directory traversal attack vulnerability
| From: | cmb@php.net | Date: | Mon, 18 Jul 2016 12:39:54 +0000 |
| Subject: | Sec Bug->Doc #72608 [Opn->Dup]: Example on manual page includes a directory traversal attack vulnerability | ||
| References: | 1 | Groups: | php.doc.bugs |
| Request: | Send a blank email to doc-bugs+get-13736@lists.php.net to get a copy of this message | ||
Edit report at https://bugs.php.net/bug.php?id=72608&edit=1
ID: 72608
Updated by: cmb@php.net
Reported by: adrian dot testaavila at gmail dot com
Summary: Example on manual page includes a directory
traversal attack vulnerability
-Status: Open
+Status: Duplicate
-Type: Security
+Type: Documentation Problem
Package: Documentation problem
Operating System: n/a
PHP Version: Irrelevant
-Assigned To:
+Assigned To: cmb
Block user comment: N
Private report: N
New Comment:
Duplicate of bug #72609.
Previous Comments:
------------------------------------------------------------------------
[2016-07-17 15:25:53] 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
------------------------------------------------------------------------
--
Edit this bug report at https://bugs.php.net/bug.php?id=72608&edit=1