This is probably a simple question, however I am finding it difficult to accomplish.
I have a php class ("class.upload.php" code is below*) that is called with:
<?php
$upload = new upload();
$upload->upload_file();
?>
so in a test page it would be like so:
<?php
$upload = new upload();
$upload->upload_file();
?>
<form action="" method="post" enctype="multipart/form-data">
<input type="file" id="real_upload" class="hide" name="file" />
<input type="submit" id="real_submit" class="hide" value="Upload" />
</form>
The problem is I'm using this upload system for parts of the website, what I want is to have this class upload the file once it has gone through the "gallery upload" segment:
<?php
$mysql_link = mysql_connect("localhost", "", "");
mysql_select_db("") or die("Could not select database");
while($counter <= count($photos_uploaded)) {
if($photos_uploaded['size'][$counter] > 0) {
if(!array_key_exists($photos_uploaded['type'][$counter], $known_photo_types)) {
$result_final .= "File ".($counter+1)." is not a photo<br />";
}
else {
mysql_query( "INSERT INTO gallery_photos(`photo_filename`, `photo_caption`, `photo_category`) VALUES('0', '".addslashes($photo_caption[$counter])."', '".addslashes($_POST['category'])."')" );
$new_id = mysql_insert_id();
$filetype = $photos_uploaded['type'][$counter];
$extention = $known_photo_types[$filetype];
$filename = $new_id.".".$extention;
mysql_query( "UPDATE gallery_photos SET photo_filename='".addslashes($filename)."' WHERE photo_id='".addslashes($new_id)."'" );
}
?>
How would i approach this?
Thanks, Keiran
class.upload.php
<?php
class Upload {
//File Max Size:
protected $max_file_size = 5;
public function upload_file() {
//Check for upload request:
if(isset($_FILES['file'])) {
//Set File Information:
$file = array(
'name' => $_FILES['file']['name'],
'type' => $_FILES['file']['type'],
'size' => $_FILES['file']['size'],
'temp' => $_FILES['file']['tmp_name'],
'error' => $_FILES['file']['error']
);
//Check if it is under the max size limit
if($file['size'] < ($this->max_file_size * 1048576)) {
//Filename:
$filename = strtolower($file['name']);
$filename = str_replace(" ","_",$filename);
//Check for a custom path location, if none exists it will load into a file associated directory
if (isset($_REQUEST['customPath'])) {
$path = 'uploads/'.$_REQUEST['customPath'].'/';
} else {
$path = 'uploads/'.$file['type'].'/';
if(!file_exists($path) && !is_dir($path)) {
mkdir($path, 开发者_StackOverflow中文版"511", true);
}
}
//Now lets more the file:
$move_file = move_uploaded_file($file['temp'], $path . $filename);
if($move_file) {
echo 'uploads/'.$filename;
}
} else {
echo 'Your file is too big to upload to our server.';
}
}
}
}
?>
Your upload class is opening your server to a complete compromise.
- You do not sanitize the
customPath
field, and blindly use it as part of the "where do I put this file" path. A malicious user can enter a relative path and your script will happily attempt to write the uploaded file ANYWHERE on your server - You do not check for filename collisions, so a malicious user can combine vulnerability #1 with this and your script will happily overwrite any file the webserver has access to. While (I hope) your server isn't running as root, consider a customPath of
../../../../../../../../../../etc/
and an uploaded filename ofpasswd
, leading your webserver to overwrite/etc/passwd
with a user-supplied version. - The
size
parameter in the $_FILES array is also user-supplied and can be trivially forged. A malicious user can set it to1
and upload a multi-terabyte file if they so choose. It is better to determine actual filesize withfilesize($_FILES['file']['tmp_name'])
instead. - Checking
isset($_FILES['file'])
is not sufficient to determine if a file was actually uploaded. Thefile
entry will be recreated ANY TIME an<input type="file" name="file">
is present within the form, whether the upload succeeded or not. You have to check($_FILES['file']['error'] === UPLOAD_ERR_OK)
to see if the upload was sucessful or not (the error constants are defined here) - The
type
portion of the $_FILES array is also user-supplied and can be subverted. Nothing prevents a malicious user from setting it toimage/jpeg
but still uploadingnasty_virus_from_hell.exe
to your server. You have to usegetimagesize()
orFileInfo()
to determine the mime type yourself in a secure manner. addslashes()
is a very poor method of sanitizing data for SQL query insertion. It is NOT a secure method, usemysql_real_escape_string()
, which uses MySQL's own API to do sanitization, and do so in a way guaranteed to be "safe" for MySQL.
精彩评论