Sanitize variables on form submit

Question

i have this code and i would like to know if I'm sanitising my code correctly (user input) i am practicing my security coding for a system that i am working on and i would like to know if i am doing thing the right way.

If there is any thing that i can improve to get things more solid i would very much like to know about them.

UPDATE

after sum reading i have changed my connection to PDO and if i am understand currently i dont need to sanitize the query?

<?php
require_once 'app/helpers.php';
session_start();
$error = '';


if($_POST){


    
 $itemtype = filter_input(INPUT_POST, 'itemtype', FILTER_SANITIZE_STRING);
 $itemtype = trim($itemtype);
    $display = filter_input(INPUT_POST, 'itemdisplay', FILTER_SANITIZE_STRING);
    $display = trim($display);
    $brand = filter_input(INPUT_POST, 'brand', FILTER_SANITIZE_STRING);
$brand = trim($brand);
    $model = filter_input(INPUT_POST, 'model', FILTER_SANITIZE_STRING);
    $model = trim($model);
    $spec = filter_input(INPUT_POST, 'spec', FILTER_SANITIZE_STRING);
    $spec = trim($spec);
    $sn = filter_input(INPUT_POST, 'sn', FILTER_SANITIZE_STRING);
     $sn = trim($sn);
    $setname =  filter_input(INPUT_POST, 'setname', FILTER_SANITIZE_STRING);
    $setname = trim($setname);
    $itemstat =  filter_input(INPUT_POST, 'itemstat', FILTER_SANITIZE_STRING);
    $itemstat = trim($itemstat);



    if(empty($itemtype)){
        $error = '<div class="alert alert-danger alert-dismissable">
					<button type="button" class="close" data-dismiss="alert" aria-hidden="true">×</button> תכניס את הפריט לקבוצה לא יפה! </div>';
    }elseif (empty($display)){
        $error = '<div class="alert alert-danger alert-dismissable">
					<button type="button" class="close" data-dismiss="alert" aria-hidden="true">×</button> אם לא נציג ניתן לו שם איך יקחו אותו? </div>';
     }elseif (empty($brand)){
        $error = '<div class="alert alert-danger alert-dismissable">
					<button type="button" class="close" data-dismiss="alert" aria-hidden="true">×</button> סליחה... מי יצר את הפריט? </div>';
    }elseif (empty($model)){
        $error = '<div class="alert alert-danger alert-dismissable">
					<button type="button" class="close" data-dismiss="alert" aria-hidden="true">×</button> רגע...איזה דגם זה? </div>';
    }elseif (empty($spec)){
        $error = '<div class="alert alert-danger alert-dismissable">
					<button type="button" class="close" data-dismiss="alert" aria-hidden="true">×</button> לא מגיע שתכתוב עליו כמה מילים? </div>';
    }elseif (empty($sn)){
        $error = '<div class="alert alert-danger alert-dismissable">
					<button type="button" class="close" data-dismiss="alert" aria-hidden="true">×</button> מספר סידורי זה הכי אחי (ושלא יהיה אותו דבר כמו של פריט אחר...לא נעים..) </div>';
     }elseif (empty($setname)){
        $error = '<div class="alert alert-danger alert-dismissable">
					<button type="button" class="close" data-dismiss="alert" aria-hidden="true">×</button>  אני חייב להיות בזוגיות...מה שם הסט שלי? </div>';
   }elseif (empty($itemstat)){
        $error = '<div class="alert alert-danger alert-dismissable">
					<button type="button" class="close" data-dismiss="alert" aria-hidden="true">×</button>    לאחרונה סיימתי קשר רציני... מה הסטטוס שלי? </div>';
    }else{
        if(!empty(filter_input(INPUT_POST, 'name', FILTER_SANITIZE_STRING)) || !empty(filter_input(INPUT_POST, 'email', FILTER_SANITIZE_STRING)) || !empty($_FILES['file']['name'])) {

            $uploadedFile = '';
            if (!empty($_FILES["file"]["type"])) {
                $fileName = $_FILES['file']['name'];
                $valid_extensions = array("jpeg", "jpg", "png");
                $temporary = explode(".", $_FILES["file"]["name"]);
                $file_extension = end($temporary);
                if ((($_FILES["file"]["type"] == "image/jpg") || ($_FILES["file"]["type"] == "image/jpeg")) && in_array($file_extension, $valid_extensions)) {
                    $sourcePath = $_FILES['file']['tmp_name'];
                    $targetPath = "items-img/" . $fileName;
                    if (move_uploaded_file($sourcePath, $targetPath)) {
                        
                        $uploadedFile = $fileName ;
                       
                    }
                }
            }

        }
      $stm = $link -> prepare("INSERT INTO item (item_desc,display,brand,model,spec,sn,set_name,status,item_pic) VALUES ('$itemtype','$display','$brand','$model','$spec','$sn','$setname','$itemstat','$uploadedFile')");

$stm->execute(array('item_desc' => $itemtype , 'display' => $display ,'brand' => $brand ,'model' => $model ,'item_desc' => $itemtype ,'spec' => $spec ,
    'sn' => $sn ,'set_name' => $setname ,'status' => $itemstat ,'name' => $uploadedFile ));
 
        $error = '<div class="alert alert-success alert-dismissable">
					<button type="button" class="close" data-dismiss="alert" aria-hidden="true">×</button>    יש לנו פריט חדש! </div>';


}
}
?>


<div>
    <?= $error ?>

</div>

Please learn about SQL injection and prepared statement, your query is wide open — Jules R
– Jules R, Commented Dec 19, 2018 at 9:00
codereview.stackexchange.com is where this question should be posted — Isaac
– Isaac, Commented Dec 19, 2018 at 9:00
No problem. Posted an answer on here for you anyway to save you some time. I'd still suggest posting it on there though (after using prepared statements) so they give you more in depth feedback. Also changed some of the readability of your question so it should be easier for people to read:D — Isaac
– Isaac, Commented Dec 19, 2018 at 9:08

Isaac · Accepted Answer · 2018-12-19 09:07:19Z

2

You're sanitising user input and escaping strings. This alone isn't secure. Take a look at using prepared statements. http://php.net/manual/en/mysqli.prepare.php

This means that you prepare the SQL statement, then bind your parameters to that statement, then it is executed and sent to your Dbms. A really good explanation of this (with pretty diagrams) is on the third answer down on this thread. How does a PreparedStatement avoid or prevent SQL injection?

Example code taken from first thread. Try to break it down and understand what it means. It's easy to follow and easy to implement.

function secured_signup($username,$password)
{    
$connection = new mysqli($dbhost,$dbusername,$dbpassword,$dbname);    
if ($connection->connect_error) 
die("Secured");

$prepared = $connection->prepare("INSERT INTO `users` ( `username` , `password`    ) VALUES ( ? , ? ) ; ");
if($prepared==false)
die("Secured");

$result=$prepared->bind_param("ss",$username,$password);
if($result==false)
die("Secured");

$result=$prepared->execute();    
if($result==false)
die("Secured");

$prepared->close();
$connection->close();    
}

Hopefully this helps you on this thread. But I'd still suggest posting it to code review on the stack exchange

answered Dec 19, 2018 at 9:07

Isaac

81411 silver badges23 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Isaac Over a year ago

@drorshalit If used correctly you don't need to sanitise input with PDO. Here's a good thread detailing why stackoverflow.com/questions/4364686/…

Collectives™ on Stack Overflow

Sanitize variables on form submit

1 Answer 1

1 Comment

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

1 Comment

Your Answer

Sign up or log in

Post as a guest

Linked

Related