cobusbo Posted August 26, 2014 Share Posted August 26, 2014 Hi Is there a way I can change my script to auto name the file when its being stored? Maybe by numbers lets say each time I upload a picture it should rename it to the number after the previous number. like example" Upload 1: 1.jpg Upload 2: 2.jpg etc.. My current code I have is as follow Form.html <html> <body><br> <h1>My upload file form.</h1> <form enctype="multipart/form-data" action="process.php" method="post"> Enter File Name:<input type="text" name="name" value="Please enter file name:" /><br> Enter Description:<input type="text" name="desc" value="Description:" /><br> Select image:<input type="file" name="prodImg"><br> <input type="submit" value="Submit Page" /> </form> <?php include "menu3.php"; ?> </body> </html> And my processing script process.php <?php extract($_POST); $name; $desc; $fileType = $_FILES['prodImg']['type']; $fileSize = $_FILES['prodImg']['size']; if($fileSize/1024 > '150') { echo 'Filesize is not correct it should equal to 2 MB or less than 2 MB.'; exit(); } //FileSize Checking if($fileType != 'image/gif' && $fileType != 'image/jpg' && $fileType != 'image/jpeg' ) { echo 'Sorry this file type is not supported we accept only. Jpeg, Gif, PNG, or '; exit(); } //file type checking ends here. $upFile = 'files/galaxywars/'.date('Y_m_d_H_i_s').$_FILES['prodImg']['name']; if(is_uploaded_file($_FILES['prodImg']['tmp_name'])) { if(!move_uploaded_file($_FILES['prodImg']['tmp_name'], $upFile)) { echo 'Problem could not move file to destination. Please check again later. <a href="index.php">Please go back.</a>'; exit; } } else { echo 'Problem: Possible file upload attack. Filename: '; echo $_FILES['prodImg']['name']; exit; } $prodImg = $upFile; //File upload ends here. $upFile; echo "Thank you for sending your file"; ?> <?php include "menu3.php"; ?> Can anyone assist me with this problem please? Quote Link to comment Share on other sites More sharing options...
Jacques1 Posted August 26, 2014 Share Posted August 26, 2014 We've already discussed this problem dozens of times, the last thread is just a few items below yours. I generally recommend random filenames, because they're foolproof and don't expose the upload behaviour of your users. However, if you insist on numbering the files, you should use an AUTO_INCREMENT column in your database. You'll have to store the original filename, anyway. So insert the metadata of the file into your database, retrieve the ID of the inserted row and use that as the filename. Besides this, your code has some major security issues. You check the MIME type in the $_FILES array, but this information comes from the user and can be anything they want. For example, I could upload a malicious PHP script, declare it as a JPEG image, and you would happily accept it. Checking the file type actually doesn't protect you at all, because even a perfectly valid image may very well contain malicious code. What matters is how the file is interpreted. As long as your webserver treats the file as an image, you're safe, no matter what the actual file content is. However, if that same file is treated as a script (due to a “.php” extension, for example), it may suddenly turn into malware and attack your server. So that's what you need to worry about. Never accept the user-provided file extension. Always set the extension yourself. Make sure there's only one extension. Turn off script execution in the upload folder. Quote Link to comment Share on other sites More sharing options...
cobusbo Posted August 26, 2014 Author Share Posted August 26, 2014 We've already discussed this problem dozens of times, the last thread is just a few items below yours. I generally recommend random filenames, because they're foolproof and don't expose the upload behaviour of your users. However, if you insist on numbering the files, you should use an AUTO_INCREMENT column in your database. You'll have to store the original filename, anyway. So insert the metadata of the file into your database, retrieve the ID of the inserted row and use that as the filename. Besides this, your code has some major security issues. You check the MIME type in the $_FILES array, but this information comes from the user and can be anything they want. For example, I could upload a malicious PHP script, declare it as a JPEG image, and you would happily accept it. Checking the file type actually doesn't protect you at all, because even a perfectly valid image may very well contain malicious code. What matters is how the file is interpreted. As long as your webserver treats the file as an image, you're safe, no matter what the actual file content is. However, if that same file is treated as a script (due to a “.php” extension, for example), it may suddenly turn into malware and attack your server. So that's what you need to worry about. Never accept the user-provided file extension. Always set the extension yourself. Make sure there's only one extension. Turn off script execution in the upload folder. I'm aware of the security problems, i've buried this this for deep in subfolders and is for my own use. Im not using any database... Is there maybe a way to do it without a database present? Quote Link to comment Share on other sites More sharing options...
CroNiX Posted August 26, 2014 Share Posted August 26, 2014 Probably the most efficient way would be to have your script create a text file and store the last number used. That way you could just quickly read the file to determine what the next number should be to rename the new file, and also overwrite the old number with the new one. Otherwise you'd have to do something like scan the dirs and do comparisons to see what the highest number is, which would get slower and slower as you add more images. Quote Link to comment Share on other sites More sharing options...
Jacques1 Posted August 26, 2014 Share Posted August 26, 2014 I'm aware of the security problems, i've buried this this for deep in subfolders and is for my own use. That's no excuse for bad code, is it? Im not using any database... Is there maybe a way to do it without a database present? Like I said: random numbers. Probably the most efficient way would be to have your script create a text file and store the last number used. This is a PITA if done correctly. It's not enough to simply increment some counter. You also have to synchronize file access to that simultaneous uploads don't read the same counter value and overwrite each other. Quote Link to comment Share on other sites More sharing options...
cobusbo Posted August 26, 2014 Author Share Posted August 26, 2014 The reason I want it to use numbers is because I use a gallery script that show the images in descending order. So every time I upload a picture I want it be to be the latest shown image on my gallery... And doing it manually takes a lot of time... Quote Link to comment Share on other sites More sharing options...
CroNiX Posted August 26, 2014 Share Posted August 26, 2014 This is a PITA if done correctly. It's not enough to simply increment some counter. You also have to synchronize file access to that simultaneous uploads don't read the same counter value and overwrite each other. Yes, but he said for his own use so it didn't sound like that would be an issue. I have an invoicing app that I created that runs on my local machine and I am the only one who accesses it. If I was coding it for multiuser use I would have coded it differently than I did. There is 0 security in it as I'm the only one who uses it. I do agree with your points. Quote Link to comment Share on other sites More sharing options...
cobusbo Posted August 26, 2014 Author Share Posted August 26, 2014 Yes, but he said for his own use so it didn't sound like that would be an issue. I have an invoicing app that I created that runs on my local machine and I am the only one who accesses it. If I was coding it for multiuser use I would have coded it differently than I did. There is 0 security in it as I'm the only one who uses it. I do agree with your points. Any Example's to point me in the right direction? Quote Link to comment Share on other sites More sharing options...
Jacques1 Posted August 26, 2014 Share Posted August 26, 2014 (edited) I'm not talking about a multi-user upload. You have the exact same problem if you upload multiple files, use tabbed browsing or whatever. Any situation which results in two uploads in quick succession can break your system. The belief that private applications don't need any security is also somewhat problematic. First of all, who says the application is actually private? Is it online? Are you sure that your authentication layer is absolutely perfect? Secondly, a lot of the things which people call “security” is much more than that. It's about making sure that the application works correctly under all circumstances. This is needed in any case, regardless of whether the application is used by you, your friends or the whole world. So we have no database and no ambition? Then I'd use a synchronized counter: <?php ini_set('display_errors', 1); $filename = null; $counter_path = '/path/to/counter'; $counter_fp = fopen($counter_path, 'r+'); if ($counter_fp) { if (flock($counter_fp, LOCK_EX)) { // read current value $current_index = fread($counter_fp, filesize($counter_path)); // increment counter ftruncate($counter_fp, 0); rewind($counter_fp); if (fwrite($counter_fp, $current_index + 1)) { $filename = $current_index; } else { trigger_error('Failed to increment counter.'); } flock($counter_fp, LOCK_UN); } else { trigger_error('Failed to lock counter file.'); } fclose($counter_fp); } else { trigger_error('Failed to open counter file.'); } if ($filename) { // now we can use the counter value } Like I said, it's a PITA. And if anything goes wrong when the counter is incremented, we lose the value entirely. Edited August 26, 2014 by Jacques1 Quote Link to comment Share on other sites More sharing options...
cobusbo Posted August 26, 2014 Author Share Posted August 26, 2014 I'm not talking about a multi-user upload. You have the exact same problem if you upload multiple files, use tabbed browsing or whatever. Any situation which results in two uploads in quick succession can break your system. The belief that private applications don't need any security is also somewhat problematic. First of all, who says the application is actually private? Is it online? Are you sure that your authentication layer is absolutely perfect? Secondly, a lot of the things which people call “security” is much more than that. It's about making sure that the application works correctly under all circumstances. This is needed in any case, regardless of whether the application is used by you, your friends or the whole world. So we have no database and no ambition? Then I'd use a synchronized counter: <?php ini_set('display_errors', 1); $filename = null; $counter_path = '/path/to/counter'; $counter_fp = fopen($counter_path, 'r+'); if ($counter_fp) { if (flock($counter_fp, LOCK_EX)) { // read current value $current_index = fread($counter_fp, filesize($counter_path)); // increment counter ftruncate($counter_fp, 0); rewind($counter_fp); if (fwrite($counter_fp, $current_index + 1)) { $filename = $current_index; } else { trigger_error('Failed to increment counter.'); } flock($counter_fp, LOCK_UN); } else { trigger_error('Failed to lock counter file.'); } fclose($counter_fp); } else { trigger_error('Failed to open counter file.'); } if ($filename) { // now we can use the counter value } Like I said, it's a PITA. And if anything goes wrong when the counter is incremented, we lose the value entirely. Ok as you said above this could cause bigger trouble for me in the future... Would you guys recommend me to use a upload script that is connected via MySQL? If so where can I find a good example... I'm a rookie at this, but im trying my best to solve the problem. The main use for it is the following: Upload .jpg images to specific directory - Mustn't create thumbs etc... in the same folder Max file size limit 150kb. Must auto increase number as file name. The only reason I used the above script were because its the only good example I could find and is very simple to setup. Quote Link to comment Share on other sites More sharing options...
CroNiX Posted August 26, 2014 Share Posted August 26, 2014 The belief that private applications don't need any security is also somewhat problematic. First of all, who says the application is actually private? Is it online? Are you sure that your authentication layer is absolutely perfect? Secondly, a lot of the things which people call “security” is much more than that. It's about making sure that the application works correctly under all circumstances. This is needed in any case, regardless of whether the application is used by you, your friends or the whole world. Sorry, not meaning to hijack the thread but wanted to answer you. My invoicing app runs only on my internal home network, not publicly accessible to anyone but myself. So, no user auth or actual security of any kind. It's well written and have had no issues in the last 5 years of using it. Quote Link to comment Share on other sites More sharing options...
cobusbo Posted August 26, 2014 Author Share Posted August 26, 2014 (edited) Ok I made the switch to Mysql upload form <?php include('config.php'); if (!isset($_FILES['image']['tmp_name'])) { echo ""; }else{ $file=$_FILES['image']['tmp_name']; $image= addslashes(file_get_contents($_FILES['image']['tmp_name'])); $image_name= addslashes($_FILES['image']['name']); move_uploaded_file($_FILES["image"]["tmp_name"],"photos/" . $_FILES["image"]["name"]); $location="photos/" . $_FILES["image"]["name"]; $caption=$_POST['caption']; $save=mysql_query("INSERT INTO photos (location, caption) VALUES ('$location','$caption')"); header("location: index.php"); exit(); } ?> My SQL looks like CREATE TABLE IF NOT EXISTS `photos` ( `id` int(11) NOT NULL AUTO_INCREMENT, `location` varchar(100) NOT NULL, `caption` varchar(100) NOT NULL, PRIMARY KEY (`id`) ) ENGINE=MyISAM DEFAULT CHARSET=latin1 AUTO_INCREMENT=5 ; So ID must be the field I should rename my photo upload to... so how would I change the line... And another question is the caption and location option can be removed from the database since it will be unnecessary Edited August 26, 2014 by cobusbo Quote Link to comment Share on other sites More sharing options...
Jacques1 Posted August 26, 2014 Share Posted August 26, 2014 It's great that you're using a proper database, but it's not really a good idea to copy and paste random code you found somewhere on the Internet. The code is pretty much the worst you can get; it's out of date since at least a decade, and now you indeed have no security whatsoever. Write your own code. I know it's tedious and time-consuming, but that's simply how programming works. Create a table for all the information you need, and then use PDO to insert your data. It's no rocket science. Sorry, not meaning to hijack the thread but wanted to answer you. My invoicing app runs only on my internal home network, not publicly accessible to anyone but myself. So, no user auth or actual security of any kind. It's well written and have had no issues in the last 5 years of using it. The “funny” thing is that it's actually fairly easy to attack a local application without direct access to the network itself. It's enough that you have access and use the Internet on the same machine. For example, the local user interface of many routers used to be vulnerable to cross-site request forgery: The attackers didn't need access to the interface, they simply made the victim (or the victim's browser) take action on their behalf. I don't know whether or not this applies in your case as well, and that's not the topic. My point is simply that secure and robust code is not only about protecting some public website from evil hackers. You may very well need it on your local network as well. The only theoretical exception is when you have an entirely isolated network which isn't connected to the Internet in any way. Quote Link to comment Share on other sites More sharing options...
cobusbo Posted August 26, 2014 Author Share Posted August 26, 2014 Is this any better? My current code is as follow <?php #################################################################### # File Upload Form 1.1 #################################################################### # #################################################################### #################################################################### # SETTINGS START #################################################################### // Folder to upload files to. Must end with slash / define('DESTINATION_FOLDER','/www/zubrag/tmp/'); // Maximum allowed file size, Kb // Set to zero to allow any size define('MAX_FILE_SIZE', 0); // Upload success URL. User will be redirected to this page after upload. define('SUCCESS_URL','http://www.example.com/upload-success.html'); // Allowed file extensions. Will only allow these extensions if not empty. // Example: $exts = array('avi','mov','doc'); $exts = array(); // rename file after upload? false - leave original, true - rename to some unique filename define('RENAME_FILE', true); // put a string to append to the uploaded file name (after extension); // this will reduce the risk of being hacked by uploading potentially unsafe files; // sample strings: aaa, my, etc. define('APPEND_STRING', ''); // Need uploads log? Logs would be saved in the MySql database. define('DO_LOG', true); // MySql data (in case you want to save uploads log) define('DB_HOST','localhost'); // host, usually localhost define('DB_DATABASE','mydb'); // database name define('DB_USERNAME','myusername'); // username define('DB_PASSWORD','password-here'); // password /* NOTE: when using log, you have to create mysql table first for this script. Copy paste following into your mysql admin tool (like PhpMyAdmin) to create table If you are on cPanel, then prefix _uploads_log on line 205 with your username, so it would be like myusername_uploads_log CREATE TABLE _uploads_log ( log_id int(11) unsigned NOT NULL auto_increment, log_filename varchar(128) default '', log_size int(10) default 0, log_ip varchar(24) default '', log_date timestamp, PRIMARY KEY (log_id), KEY (log_filename) ); */ #################################################################### ### END OF SETTINGS. DO NOT CHANGE BELOW #################################################################### // Allow script to work long enough to upload big files (in seconds, 2 days by default) @set_time_limit(172800); // following may need to be uncommented in case of problems // ini_set("session.gc_maxlifetime","10800"); function showUploadForm($message='') { $max_file_size_tag = ''; if (MAX_FILE_SIZE > 0) { // convert to bytes $max_file_size_tag = "<input name='MAX_FILE_SIZE' value='".(MAX_FILE_SIZE*1024)."' type='hidden' >\n"; } // Load form template include ('file-upload.html'); } // errors list $errors = array(); $message = ''; // we should not exceed php.ini max file size $ini_maxsize = ini_get('upload_max_filesize'); if (!is_numeric($ini_maxsize)) { if (strpos($ini_maxsize, 'M') !== false) $ini_maxsize = intval($ini_maxsize)*1024*1024; elseif (strpos($ini_maxsize, 'K') !== false) $ini_maxsize = intval($ini_maxsize)*1024; elseif (strpos($ini_maxsize, 'G') !== false) $ini_maxsize = intval($ini_maxsize)*1024*1024*1024; } if ($ini_maxsize < MAX_FILE_SIZE*1024) { $errors[] = "Alert! Maximum upload file size in php.ini (upload_max_filesize) is less than script's MAX_FILE_SIZE"; } // show upload form if (!isset($_POST['submit'])) { showUploadForm(join('',$errors)); } // process file upload else { while(true) { // make sure destination folder exists if (!@file_exists(DESTINATION_FOLDER)) { $errors[] = "Destination folder does not exist or no permissions to see it."; break; } // check for upload errors $error_code = $_FILES['filename']['error']; if ($error_code != UPLOAD_ERR_OK) { switch($error_code) { case UPLOAD_ERR_INI_SIZE: // uploaded file exceeds the upload_max_filesize directive in php.ini $errors[] = "File is too big (1)."; break; case UPLOAD_ERR_FORM_SIZE: // uploaded file exceeds the MAX_FILE_SIZE directive that was specified in the HTML form $errors[] = "File is too big (2)."; break; case UPLOAD_ERR_PARTIAL: // uploaded file was only partially uploaded. $errors[] = "Could not upload file (1)."; break; case UPLOAD_ERR_NO_FILE: // No file was uploaded $errors[] = "Could not upload file (2)."; break; case UPLOAD_ERR_NO_TMP_DIR: // Missing a temporary folder $errors[] = "Could not upload file (3)."; break; case UPLOAD_ERR_CANT_WRITE: // Failed to write file to disk $errors[] = "Could not upload file (4)."; break; case 8: // File upload stopped by extension $errors[] = "Could not upload file (5)."; break; } // switch // leave the while loop break; } // get file name (not including path) $filename = @basename($_FILES['filename']['name']); // filename of temp uploaded file $tmp_filename = $_FILES['filename']['tmp_name']; $file_ext = @strtolower(@strrchr($filename,".")); if (@strpos($file_ext,'.') === false) { // no dot? strange $errors[] = "Suspicious file name or could not determine file extension."; break; } $file_ext = @substr($file_ext, 1); // remove dot // check file type if needed if (count($exts)) { /// some day maybe check also $_FILES['user_file']['type'] if (!@in_array($file_ext, $exts)) { $errors[] = "Files of this type are not allowed for upload."; break; } } // destination filename, rename if set to $dest_filename = $filename; if (RENAME_FILE) { $dest_filename = md5(uniqid(rand(), true)) . '.' . $file_ext; } // append predefined string for safety $dest_filename = $dest_filename . APPEND_STRING; // get size $filesize = intval($_FILES["filename"]["size"]); // filesize($tmp_filename); // make sure file size is ok if (MAX_FILE_SIZE > 0 && MAX_FILE_SIZE*1024 < $filesize) { $errors[] = "File is too big (3)."; break; } if (!@move_uploaded_file($tmp_filename , DESTINATION_FOLDER . $dest_filename)) { $errors[] = "Could not upload file (6)."; break; } if (DO_LOG) { // Establish DB connection $link = @mysql_connect(DB_HOST, DB_USERNAME, DB_PASSWORD); if (!$link) { $errors[] = "Could not connect to mysql."; break; } $res = @mysql_select_db(DB_DATABASE, $link); if (!$res) { $errors[] = "Could not select database."; break; } $m_ip = mysql_real_escape_string($_SERVER['REMOTE_ADDR']); $m_size = $filesize; $m_fname = mysql_real_escape_string($dest_filename); $sql = "insert into _uploads_log (log_filename,log_size,log_ip) values ('$m_fname','$m_size','$m_ip')"; $res = @mysql_query($sql); if (!$res) { $errors[] = "Could not run query."; break; } @mysql_free_result($res); @mysql_close($link); } // if (DO_LOG) // redirect to upload success url header('Location: ' . SUCCESS_URL); die(); break; } // while(true) // Errors. Show upload form. $message = join('',$errors); showUploadForm($message); } ?> I know I should change the lines // destination filename, rename if set to $dest_filename = $filename; if (RENAME_FILE) { $dest_filename = md5(uniqid(rand(), true)) . '.' . $file_ext; and if (!@move_uploaded_file($tmp_filename , DESTINATION_FOLDER . $dest_filename)) { $errors[] = "Could not upload file (6)."; break; Im not sure how to implement the changes to select from database and to implement it into here and I heard the while(true) is wrong how should it be like then? Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.