magcr23 Posted July 7, 2015 Share Posted July 7, 2015 Hi guys, I have this code: <head> <meta charset="utf-8"> </head> <body oncontextmenu="return false"> <?php include("conexao.php"); include("funcao.php"); @$id = limpa($_POST["id"]); @$preco = limpa($_POST["novopreco"]); @$descricao = mysql_real_escape_string($_POST["novodesc"]); @$data = date('Y-m-d'); //PASTA PARA MOVER OS FICHEIROS @$destination = "image/"; if(isset($_FILES['files2'])){ if(!empty($preco)){ // INSERIR DADOS NA BD $query2= "UPDATE casa SET preco = '$preco' WHERE id = '$id'"; mysqli_query($con, $query2); } if(!empty($descricao)){ // INSERIR DADOS NA BD $query2= "UPDATE casa SET descricao = '$descricao' WHERE id = '$id'"; mysqli_query($con, $query2); } foreach($_FILES['files2']['tmp_name'] as $key => $tmp_name){ //NOME DO FICHEIRO @$value = $_FILES['files2']['name'][$key]; //LISTA DE EXTENÇÕES PERMITIDAS @$allowed = array('PNG','JPEG','JPG'); //VER QUAL É A EXTENÇÃO @$exts = explode("." , $value); @$info = getimagesize($destination . $value); //SE A EXTENÇÃO DO FICHEIRO NÃO ESTIVER NA LISTA DAS PERMITIDAS if(@!in_array($exts[1], $allowed)){ //echo 'extensao invalida <br/>'; //print_r( 'Ficheiro-->' . $value . '<br/>Extencao-->' . @$exts[1] . '<br/>Apenas imagens sao permitidas'); }else{ //SE A EXTENÇÃO DO FICHEIRO ESTIVER NA LISTA DAS PERMITIDAS //echo 'Ficheiro valido<br/>'; $type = $_FILES['files2']['type'][$key]; $size = $_FILES['files2']['size'][$key]; $error = $_FILES['files2']['error'][$key]; $tmp_name = $_FILES['files2']['tmp_name'][$key]; /*-------------------------------------------------------------*/ //VARIAVEIS PARA ALTERAR NOME (adicionar um numero a frente(1,2,3,4,5...) $i = 1; $actual_name = pathinfo($value,PATHINFO_FILENAME); $original_name = $actual_name; $extension = pathinfo($value, PATHINFO_EXTENSION); /*-------------------------------------------------------------*/ //REDIMENSIONAR IMAGEM $uploadedfile = $tmp_name; if($extension=="jpg" || $extension=="jpeg" || $extension=="JPG" || $extension=="JPEG" ){ echo 'jpg -- '.$value; $src = imagecreatefromjpeg($uploadedfile); } if($extension=="png" || $extension=="PNG"){ echo 'png'; $src = imagecreatefrompng($uploadedfile); } list($width, $height) = getimagesize($uploadedfile); $newWidth = 500; $newHeight = ($width / $height) * $newWidth; // VERIFICAR SE É UMA IMAGEM VÁLIDA if($imageInfo = getimagesize($tmp_name)){ //MOVER FICHEIRO PARA PASTA DESTINO (image) if(file_exists("image/". $value)){ //SE O FICHEIRO JA EXISTE //echo 'existe<br/>'; while(file_exists('image/'.$actual_name.".".$extension)){ $actual_name = (string)$original_name.$i; $nome = $actual_name.".".$extension; $i++; } $tmp = imagecreatetruecolor($newWidth, $newHeight); imagecopyresampled($tmp, $src, 0,0,0,0, $newWidth, $newHeight, $width, $height); if($extension=="jpg" || $extension=="jpeg" || $extension=="JPG" || $extension=="JPEG" ){ imagejpeg($tmp, "image/". $nome, 100); } if($extension=="png" || $extension=="PNG"){ imagepng($tmp, "image/". $nome); } imagedestroy($src); imagedestroy($tmp); //move_uploaded_file($tmp_name,"image/".$nome); $query1 = mysqli_query($con, "INSERT INTO `imagem` (id, caminho, casa) VALUES (DEFAULT, '$nome', '$id')"); }else{ //SE O FICHEIRO NAO EXISTE //move_uploaded_file($tmp_name, "image/".$value); //CRIAR A IMAGEM $tmp = imagecreatetruecolor($newWidth, $newHeight); imagecopyresampled($tmp, $src, 0,0,0,0, $newWidth, $newHeight, $width, $height); if($extension=="jpg" || $extension=="jpeg" || $extension=="JPG" || $extension=="JPEG" ){ imagejpeg($tmp, "image/". $value, 100); } if($extension=="png" || $extension=="PNG"){ imagepng($tmp, "image/". $value); } imagedestroy($src); imagedestroy($tmp); // MOSTRA INFO DO FICHEIRO, SE FOR IMAGEM //print_r($imageInfo); $query1 = mysqli_query($con, "INSERT INTO `imagem` (id, caminho, casa) VALUES (DEFAULT, '$value', '$id')"); } } } } //header('Location: ver.php'); }else{ echo '<h1 style="color:red" align="center"><b>É necessario alterar algum condomínio!</b></h1>'; } include("fimConexao.php"); ?> </body> The problem is: If i try insert: jpg files --> it does JPG --> it don't The most awkward think is that i have the exact same code in other page and it works very well, in this page i get this error, why? Quote Link to comment Share on other sites More sharing options...
ginerjm Posted July 7, 2015 Share Posted July 7, 2015 You get WHAT error? "it does" and "it don't" do not tell us much about your problem. Quote Link to comment Share on other sites More sharing options...
Ch0cu3r Posted July 7, 2015 Share Posted July 7, 2015 Are you saying files with file extensions in capital letters are being classes as invalid files? Have you tried applying strtolower to the file extension? Quote Link to comment Share on other sites More sharing options...
CroNiX Posted July 7, 2015 Share Posted July 7, 2015 First, you are suppressing errors using the @ everywhere. That tells php NOT to display errors. But you are having problems with your code...so...you want to see the errors! Hard to tell what the problem is if you're hiding it Instead of testing for upper and lower case scenarios, why not just force the extension to lower case for your comparisons? $extension = strtolower(pathinfo($value, PATHINFO_EXTENSION)); then instead of if($extension=="jpg" || $extension=="jpeg" || $extension=="JPG" || $extension=="JPEG" ){ you could just use if($extension=="jpg" || $extension=="jpeg"){ Quote Link to comment Share on other sites More sharing options...
magcr23 Posted July 7, 2015 Author Share Posted July 7, 2015 (edited) You get WHAT error? "it does" and "it don't" do not tell us much about your problem. No errors, it just don't create the image, everything works fine less the image creation. If the extension is lower case it works, if it's UPPER CASE it returns empty CroNiX i know that i'm hidding them, i've already fixed them, it's only hided because it need to receive data from other page, and if the user just write on url the link to that page it will give some errors. Already tried what you said, and it gives me an extension error. Edited July 7, 2015 by magcr23 Quote Link to comment Share on other sites More sharing options...
CroNiX Posted July 7, 2015 Share Posted July 7, 2015 (edited) What "extension" error? You're mixing mysql and mysqli functions (mysql_real_escape_string()). Just use mysqli. Are you sure this "JPG" is actually a legitimate jpg file? You are only looking at the file extension and not the mime type. So I could just rename 'some_script.php' to 'some_image.jpg' and upload it and your script will try to process it. Or it could be a gif image renamed with a JPG extension. See this: http://www.finalwebsites.com/php-mime-type-detection/ Another problem I see is this: @$exts = explode("." , $value); and then you use it here: if(@!in_array($exts[1], $allowed)){ You assume here that the file is filename.ext (only one period) and the extension will always be after the very first dot. What if the uploaded file was named filename.small.jpg? It's a legitimate name for a jpg file but your script won't process it the way it's coded because it will look at "small" as the file type. Edited July 7, 2015 by CroNiX Quote Link to comment Share on other sites More sharing options...
ginerjm Posted July 7, 2015 Share Posted July 7, 2015 During development you should NOT be hiding any errors. YOu should be programming around them - that's what good coders do. Remove the @ signs and be sure you have php error checking turned on and see what your script might be trying to tell you. And of course your filenames could be a problem if you are looking for the wrong case in the name. Quote Link to comment Share on other sites More sharing options...
scootstah Posted July 7, 2015 Share Posted July 7, 2015 During development you should NOT be hiding any errors. YOu should be programming around them - that's what good coders do. Remove the @ signs and be sure you have php error checking turned on and see what your script might be trying to tell you. Under most circumstances, you're absolutely correct. However, in the case of getimagesize(), you have to use the @ sign unless you have a known existing file and know that the file is a valid image. Unfortunately it does not throw an exception, and there's no other way to capture the error. OP: Since you're already using getimagesize(), you checking the file extension the way you are is pointless. getimagesize() returns the MIME type of the file, which is what you should be using to check the file type anyway - the file name provided in $_FILES is considered user input, and as such you cannot trust it. getimagesize() will return boolean false if the provided file either does not exist, or is not a valid image file. You can use this to your advantage. @$info = getimagesize($destination . $value); if ($info !== false) { // the image file is valid // allowed MIME types $allowed = array('image/jpeg', 'image/png', 'image/gif'); if (in_array($info['mime'], $allowed)) { // the file is a valid MIME type } } Quote Link to comment Share on other sites More sharing options...
magcr23 Posted July 8, 2015 Author Share Posted July 8, 2015 (edited) Under most circumstances, you're absolutely correct. However, in the case of getimagesize(), you have to use the @ sign unless you have a known existing file and know that the file is a valid image. Unfortunately it does not throw an exception, and there's no other way to capture the error. OP: Since you're already using getimagesize(), you checking the file extension the way you are is pointless. getimagesize() returns the MIME type of the file, which is what you should be using to check the file type anyway - the file name provided in $_FILES is considered user input, and as such you cannot trust it. getimagesize() will return boolean false if the provided file either does not exist, or is not a valid image file. You can use this to your advantage. @$info = getimagesize($destination . $value); if ($info !== false) { // the image file is valid // allowed MIME types $allowed = array('image/jpeg', 'image/png', 'image/gif'); if (in_array($info['mime'], $allowed)) { // the file is a valid MIME type } } I have an question, the gold in all verifications is to the user don't upload anything to my server right? But to use getimagesize the file need's to be on the server. (i have it on the code but it's not doing anything i belive). So, what should i do? Allow any file upload and then use getimagesize? Change $destination to the temporary folder? Or am i completly wrong and miss understod the function?? Btw: I've already found the bug that wasn't allowing the upload of some files. Edited July 8, 2015 by magcr23 Quote Link to comment Share on other sites More sharing options...
CroNiX Posted July 8, 2015 Share Posted July 8, 2015 what was the bug? Quote Link to comment Share on other sites More sharing options...
magcr23 Posted July 8, 2015 Author Share Posted July 8, 2015 what was the bug? It wasn't realy an bug but a stupid thing... The allowed extencions were all on UPPER CASE: @$allowed = array('PNG','JPEG','JPG'); when it should be: @$allowed = array('PNG','JPEG','JPG', 'png', jpeg', 'jpg'); Since it don't was an error but an forgetfulness of extensions it had no errors, i wasted 13h searching for an inixistent bug. But scootstah, explode is not the best option, but i think i'll not be able to use mime from getimagesize, so i was wondering, if there's an way to explode the last ".", i think that would work... Quote Link to comment Share on other sites More sharing options...
scootstah Posted July 8, 2015 Share Posted July 8, 2015 But scootstah, explode is not the best option, but i think i'll not be able to use mime from getimagesize, so i was wondering, if there's an way to explode the last ".", i think that would work... Yes, but I already gave you the code to get the mime type from getimagesize(). What problem are you having? The way you are trying to do things will allow someone to upload anything they want to your website and potentially compromise the whole server. Don't do it. Quote Link to comment Share on other sites More sharing options...
magcr23 Posted July 8, 2015 Author Share Posted July 8, 2015 (edited) Yes, but I already gave you the code to get the mime type from getimagesize(). What problem are you having? The way you are trying to do things will allow someone to upload anything they want to your website and potentially compromise the whole server. Don't do it. It's not a problem since i don't tried to do it. Why? Because to use getimagesize i need to allow the user to upload any file to my server. And if they can upload anything probably i will have problems in the future. So, what's the best option? Allow upload any file to then i can use getimagesize? Or try something like that: (it's just an idea, i'm not using it, that's a code for test, not to the website) $str = "AAA. BBB. CCC. DDD. EEE"; $exts = explode("." , $str); $contagem = count($exts); $a = $exts[$contagem-1]; //echo $a; if(!in_array($exts[$a], $allowed)){ ... Edited July 8, 2015 by magcr23 Quote Link to comment Share on other sites More sharing options...
scootstah Posted July 8, 2015 Share Posted July 8, 2015 In order for you to be testing the file extension, the file has already been uploaded to a temporary directory, which is enough to be able to use getimagesize() on it. You are literally checking the NAME of the file, nothing more. I can rename "some-malicious-script.php" to "pretty-flowers.jpg" and completely destroy your server. See the problem with that? Quote Link to comment Share on other sites More sharing options...
magcr23 Posted July 8, 2015 Author Share Posted July 8, 2015 In order for you to be testing the file extension, the file has already been uploaded to a temporary directory, which is enough to be able to use getimagesize() on it. You are literally checking the NAME of the file, nothing more. I can rename "some-malicious-script.php" to "pretty-flowers.jpg" and completely destroy your server. See the problem with that? Yes i can see what you meen, explode won't work. I want to only allow jpg and png files, so-->$allowed = array('image/jpeg', 'image/png'); is this enough? or there are more mime tipes? like jpe.... Quote Link to comment Share on other sites More sharing options...
scootstah Posted July 8, 2015 Share Posted July 8, 2015 Yes, that's all you need. Quote Link to comment Share on other sites More sharing options...
magcr23 Posted July 8, 2015 Author Share Posted July 8, 2015 Yes, that's all you need. So i have this security: check if it's an image, check if the file name already exist in the destiny folder. Is this enough? Or should i have more? If yes, wich? Quote Link to comment Share on other sites More sharing options...
Solution scootstah Posted July 8, 2015 Solution Share Posted July 8, 2015 Typically that is all I do, but depending how you will be using the images you could still have problems. Check out this thread, where I determined it is still possible to embed code into an image and pass a MIME check: http://forums.phpfreaks.com/topic/294311-image-upload-and-risks-questions/?p=1504625 Quote Link to comment Share on other sites More sharing options...
magcr23 Posted July 8, 2015 Author Share Posted July 8, 2015 Typically that is all I do, but depending how you will be using the images you could still have problems. Check out this thread, where I determined it is still possible to embed code into an image and pass a MIME check: http://forums.phpfreaks.com/topic/294311-image-upload-and-risks-questions/?p=1504625 thx 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.