I am using PDO prepared statements to store user entered input and store that input to the database.  The code will store a text input and 5 uploaded images.Will this code protect from data SQL injection and XSS attack?  Will it be secure? If not, what are some things I should make secure?    <?php$db_username = 'sanoj';$db_password = '123456';$newname = md5(rand() * time());if (isset($_FILES['files'])) {    $uploadedFiles = array();    foreach ($_FILES['files']['tmp_name'] as $key => $tmp_name) {        $errors = array();        $file_name = md5(uniqid() . time());        $file_size = $_FILES['files']['size'][$key];        $file_tmp = $_FILES['files']['tmp_name'][$key];        $file_type = $_FILES['files']['type'][$key];        if ($file_type == image/gif) {            $sExt = .gif;        } elseif ($file_type == image/jpeg || $file_type == image/pjpeg) {            $sExt = .jpg;        } elseif ($file_type == image/png || $file_type == image/x-png) {            $sExt = .png;        }        if (!in_array($sExt, array('.gif', '.jpg', '.png'))) {            $errors[] = Image types alowed are (.gif, .jpg, .png) only!;        }        if ($file_size > 2097152000) {            $errors[] = 'File size must be less than 2 MB';        }        $desired_dir = user_data/;        if (empty($errors)) {            if (is_dir($desired_dir) == false) {                mkdir($desired_dir, 0600);        // Create directory if it does not exist            }            if (move_uploaded_file($file_tmp, $desired_dir/ . $file_name . $sExt)) {                $uploadedFiles[$key] = array($file_name . $sExt, 1);            } else {                echo Couldn't upload file  . $_FILES['files']['name'][$key];                $uploadedFiles[$key] = array($_FILES['files']['name'][$key], 0);            }        } else {        }    }    foreach ($uploadedFiles as $key => $row) {        if (!empty($row[1])) {            $codestr = '$file' . ($key + 1) . ' = $row[0];';            eval($codestr);        } else {            $codestr = '$file' . ($key + 1) . ' = NULL;';            eval($codestr);        }    }}$orig_directory = $desired_dir;    //Full image folder$thumb_directory = thumb/;    //Thumbnail folder/* Opening the thumbnail directory and looping through all the thumbs: */$dir_handle = @opendir($orig_directory); //Open Full image dirrectoryif ($dir_handle > 1) { //Check to make sure the folder opened    $allowed_types = array('jpg', 'jpeg', 'gif', 'png');    $file_type = array();    $ext = '';    $title = '';    $i = 0;    while ($file_name = @readdir($dir_handle)) {        /* Skipping the system files: */        if ($file_name == '.' || $file_name == '..')            continue;        $file_type = explode('.', $file_name);    //This gets the file name of the images        $ext = strtolower(array_pop($file_type));        /* Using the file name (withouth the extension) as a image title: */        $title = implode('.', $file_type);        $title = htmlspecialchars($title);        /* If the file extension is allowed: */        if (in_array($ext, $allowed_types)) {            /* If you would like to inpute images into a database, do your mysql query here */            /* The code past here is the code at the start of the tutorial */            /* Outputting each image: */            $nw = 100;            $nh = 100;            $source = $desired_dir{$file_name};            $stype = explode(., $source);            $stype = $stype[count($stype) - 1];            $dest = thumb/{$file_name};            $size = getimagesize($source);            $w = $size[0];            $h = $size[1];            switch ($stype) {                case 'gif':                    $simg = imagecreatefromgif($source);                    break;                case 'jpg':                    $simg = imagecreatefromjpeg($source);                    break;                case 'png':                    $simg = imagecreatefrompng($source);                    break;            }            $dimg = resizePreservingAspectRatio($simg, $nw, $nh);            imagepng($dimg, $dest);        }    }    /* Closing the directory */    @closedir($dir_handle);}try {#connection     $conn = new PDO('mysql:host=localhost;dbname=localtest', $db_username, $db_password);    $conn->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);    $data = $conn->prepare('INSERT INTO agriculture (cacat, mtype, mtitle, image1, image2, image3, image4, image5, description, mcondition, cmodel, price, youare, mname, email, phone, ylocation, ystreet) VALUES (:cacat, :mtype, :mtitle, :image1, :image2, :image3, :image4, :image5, :description, :mcondition, :cmodel, :price, :youare, :mname, :email, :phone, :ylocation, :ystreet)');    $cacat = filter_input(INPUT_POST, 'cacat', FILTER_SANITIZE_STRING, FILTER_FLAG_ENCODE_AMP);    $mtype = filter_input(INPUT_POST, 'mtype', FILTER_SANITIZE_STRING, FILTER_FLAG_ENCODE_AMP);    $mtitle = filter_input(INPUT_POST, 'mtitle', FILTER_SANITIZE_STRING, FILTER_FLAG_ENCODE_AMP);    $description = filter_input(INPUT_POST, 'description', FILTER_SANITIZE_STRING, FILTER_FLAG_ENCODE_AMP);    $mcondition = filter_input(INPUT_POST, 'mcondition', FILTER_SANITIZE_STRING, FILTER_FLAG_ENCODE_AMP);    $cmodel = filter_input(INPUT_POST, 'cmodel', FILTER_SANITIZE_STRING, FILTER_FLAG_ENCODE_AMP);    $price = filter_input(INPUT_POST, 'price', FILTER_SANITIZE_STRING, FILTER_FLAG_ENCODE_AMP);    $youare = filter_input(INPUT_POST, 'youare', FILTER_SANITIZE_STRING, FILTER_FLAG_ENCODE_AMP);    $mname = filter_input(INPUT_POST, 'mname', FILTER_SANITIZE_STRING, FILTER_FLAG_ENCODE_AMP);    $email = filter_input(INPUT_POST, 'email', FILTER_SANITIZE_STRING, FILTER_FLAG_ENCODE_AMP);    $phone = filter_input(INPUT_POST, 'phone', FILTER_SANITIZE_STRING, FILTER_FLAG_ENCODE_AMP);    $ylocation = filter_input(INPUT_POST, 'ylocation', FILTER_SANITIZE_STRING, FILTER_FLAG_ENCODE_AMP);    $ystreet = filter_input(INPUT_POST, 'ystreet', FILTER_SANITIZE_STRING, FILTER_FLAG_ENCODE_AMP);    $data->execute(array(':cacat' => $cacat,        ':mtype' => $mtype,        ':mtitle' => $mtitle,        'image1' => $file1,        'image2' => $file2,        'image3' => $file3,        'image4' => $file4,        'image5' => $file5, ':description' => $description, ':mcondition' => $mcondition, ':cmodel' => $cmodel, ':price' => $price, ':youare' => $youare, ':mname' => $mname, ':email' => $email, ':phone' => $phone, ':ylocation' => $ylocation, ':ystreet' => $ystreet));#exception handiling} catch (PDOException $e) {    echo $e->getMessage();}function resizePreservingAspectRatio($img, $targetWidth, $targetHeight) {    $srcWidth = imagesx($img);    $srcHeight = imagesy($img);    // Determine new width / height preserving aspect ratio    $srcRatio = $srcWidth / $srcHeight;    $targetRatio = $targetWidth / $targetHeight;    if (($srcWidth <= $targetWidth) && ($srcHeight <= $targetHeight)) {        $imgTargetWidth = $srcWidth;        $imgTargetHeight = $srcHeight;    } else if ($targetRatio > $srcRatio) {        $imgTargetWidth = (int) ($targetHeight * $srcRatio);        $imgTargetHeight = $targetHeight;    } else {        $imgTargetWidth = $targetWidth;        $imgTargetHeight = (int) ($targetWidth / $srcRatio);    }    // Creating new image with desired size    $targetImg = imagecreatetruecolor($targetWidth, $targetHeight);    // Add transparency if your reduced image does not fit with the new size    $targetTransparent = imagecolorallocate($targetImg, 255, 0, 255);    imagefill($targetImg, 0, 0, $targetTransparent);    imagecolortransparent($targetImg, $targetTransparent);    // Copies image, centered to the new one (if it does not fit to it)    imagecopyresampled($targetImg, $img, 0, 0, 0, 0, $targetWidth, $targetHeight, $srcWidth, $srcHeight);    return $targetImg;}?>SQLcreate table `agriculture`(`id` int(9) NOT NULL auto_increment,`cacat` varchar(255) NOT NULL default '',`mtype` varchar(255) NOT NULL default '',`mtitle` varchar(255) NOT NULL default '',`image1` varchar(255) NOT NULL default '',`image2` varchar(255) NOT NULL default '',`image3` varchar(255) NOT NULL default '',`image4` varchar(255) NOT NULL default '',`image5` varchar(255) NOT NULL default '',`description` varchar(255) NOT NULL default '',`mcondition` varchar(255) NOT NULL default '',`cmodel` varchar(255) NOT NULL default '',`price` varchar(255) NOT NULL default '',`youare` varchar(255) NOT NULL default '',`mname` varchar(255) NOT NULL default '',`email` varchar(255) NOT NULL default '',`phone` varchar(255) NOT NULL default '',`ylocation` varchar(255) NOT NULL default '',`ystreet` varchar(255) NOT NULL default '',`ipnu` varchar(255) NOT NULL default '',PRIMARY KEY (`id`)) ENGINE=MyISAM AUTO_INCREMENT=1 ;
Securing a form with image uploads
php;sql;security;pdo;sql injection
Not a complete review (do you want all the code reviewed? The comments seem to indicate that maybe not all of it is yours?), just a couple of points I noticed.Will this code protect from data SQL injection and XSS attack? Will it be secure? Your image upload code seems secure to me. A couple things I noticed:if a file cannot be uploaded, you echo the filename, which is user controlled. But an attacker cannot use a slash (I tried it with filename=test<script>alert('test');</script>.jpg, which resulted in Couldn't upload file script>.jpg). But even though an attacker cannot include an external javascript file, they can still execute arbitrary code using a file name of for example test<svg onload=alert('test')>.jpg. The only problem for an attacker to solve is how to create an error while uploading the file (the documentation says it will return an error If filename is not a valid upload file, but does not mention what that means). Still, just to be save, I would use htmlspecialchars to avoid XSS.$_FILES['files']['type'] is also user controlled, so it is generally not recommended to use it to check for illegal file extensions. If you would keep the original filename intact, this would be a major problem, but as you are completely regenerating it, this is safe. If at some point you redo your file naming process, you should keep an eye on this.you don't actually need filter_input, as you are using prepared statements, and I don't think that it actually does anything usefull against SQL injection anyways (the documentation says it Strip tags, optionally strip or encode special characters.). mkdir($desired_dir, 0700): as this is an image directory, there should be no need to execute files. 0600 should be enough.Empty Else and Error ReportingIn your upload code, you collect all error messages in an array, and then check if this array contains errors. If not, you do nothing. Instead, you should print the error messages to the user, so they know what went wrong.EvalEval is evil, so I would avoid it if I can. In your case it's safe because the input is not user controlled, but there is no reason to use it like you do. Transforming an array into X variables doesn't seem to server a purpose, just access the array directly. Or, if I am overlooking some reason why this is needed, you could also just hardcode the variables, as only the first five are actually used (you did not post your HTML code, but I'm assuming the user is aware of this).Splitting up your Code150 lines of code is too much to leave it in one big block, it would be a lot easier to read if it was split up into functions. For example uploadFile, createThumbnail, storeImage, etc.Misc$title seems to be unusedif a user does not upload a file, should the rest of the code actually execute? If not, return after asking if $_FILES is set.