Please help me to sanitize php data goint to mysql

Question

Hi friends I have a form with mysqli comnnection

<label for="fullname">Fullname</label>
<input type="text" name="fullname" />

<label for="photo">Upload photo</label>
<input name="photo" type="file"/>

and on the php ends I have

$fullname = $_POST['fullname'];

    $uploaddir = './uploads/';
    //upload file in folder
    $uploadfile = $uploaddir. basename($_FILES['photo']['name']);
    //insert filename in db
    $upload_filename = basename($_FILES['photo']['name']);
    move_uploaded_file($_FILES['photo']['tmp_name'], $uploadfile);
    $photo = $upload_filename;

$sql = "INSERT INTO members(fullname,photo) VALUES('$fullname', '$photo');
$stmt = $link->query($sql) or die($link->error);
    $stmt->close;

Please help me, I am using this on a live site

Answer 1

Perhaps you need mysql_real_escape_string() ?

http://php.net/manual/en/function.mysql-real-escape-string.php

Answer 2

I'd suggest placeholders, using an existing database library for PHP: either PDO or something from Pear, like Pear::MDB2. With placeholders you are guarranteed proper escaping by the database library, and your code becomes a little easier to read.

In such cases your code will be something like:

$sql = "INSERT INTO members (fullname,photo) VALUES (?, ?)";
$values = array($fullname, $photo) ;

$prepared = $link->prepare($sql) ;
$result = $prepared->execute($values) ;

edit I just realized you are using mysqli, so to continue using that look here or here for tutorials.

$sql = "INSERT INTO members (fullname,photo) VALUES (?, ?)";
$stmt = $link->prepare($sql) ;
//ss means the 2 parameters after it should be treated as string, string
$stmt->bind_param('ss', $fullname, $photo) ;

$result = $stmt->execute() ;

Answer 3

Use prepared statements instead:

$fullname = $_POST['fullname'];

    $uploaddir = './uploads/';
    //upload file in folder
    $uploadfile = $uploaddir. basename($_FILES['photo']['name']);
    //insert filename in db
    $upload_filename = basename($_FILES['photo']['name']);
    move_uploaded_file($_FILES['photo']['tmp_name'], $uploadfile);
    $photo = $upload_filename;

$sql = "INSERT INTO members(fullname,photo) VALUES(?, ?)";
$stmt = $link->prepare($sql);
$stmt->bind_param('ss',$fullname,$photo);
$stmt->execute();

For more information see prepared statements and parameterized queries This has the nice benifit of protecting you from SQL injection as well.

Answer 4

Watch this link, advise number 3. I suggest you to use the function escape that they recommend.

Answer 5

Seeing as you're using mysqli, you get parameterised queries. This is generally less messy than trying to escape strings yourself:

$q= $mysqli->prepare('INSERT INTO members (fullname, photo) VALUES (?, ?)');
$q->bind_param('ss', $fullname, $photo);
$q->execute();

Note that it is highly risky to trust a user-supplied filename for file uploads. In your current code, a user can upload a .php file, which, if the uploads folder is exposed to the web server, will allow an attacker to run arbitrary code of their choosing on your server.

Other potentially troublesome filenames include empty strings, non-ASCII and control characters, .htaccess on an Apache server, and files with leading/trailing dots and spaces or using one of the system reserved filenames on a Windows server. Also, there seems to be no protection against a user uploading a photo that overwrites another user's, which seems quite likely to happen by accident.

It's generally better to generate a filename from a random number or the row's primary key, and add the extension you want (eg. .jpeg). Sanitising user-supplied filenames is a much harder job than you might think.

Please help me, I am using this on a live site

If this is exposed to non-trusted users I would seriously take the site down until you have fixed it.

Answer 6

I wanted to comment on this:

Note that it is risky to trust a user-supplier filename for file uploads. In your current code, a user can upload a .php file, which, if the uploads folder is exposed to the web server, will allow an attacker to run arbitrary code of their choosing on your server.

It's generally better to generate a filename from a random number or the row's primary key, and add the extension you want (eg. .jpeg). Sanitising user-supplied filenames is a much harder job than you might think.

Sanitizing the filenames is not the right approach, IMO. For one thing, an executable can be uploaded with any filename, and will may be downloaded by other users. Many users will then try to launch that file. If it has a virus or similar, the user is now compromised.

Instead, fileinfo can be used to detect the type, unless the files are strictly images, in which case, I believe the best practice seems to be using getimagesize to check that it's an allowed format.