CODEWITHSUNDEEP
phpmysql
ktm
ktm

Reputation: 6085

Please help me to sanitize php data goint to mysql

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

Upvotes: 0

Views: 682

Answers (5)

davidtbernal
davidtbernal

Reputation: 13684

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.

Upvotes: 0

Fanis Hatzidakis
Fanis Hatzidakis

Reputation: 5340

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() ;

Upvotes: 0

bobince
bobince

Reputation: 536319

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.

Upvotes: 4

Kristoffer Sall-Storgaard
Kristoffer Sall-Storgaard

Reputation: 10636

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.

Upvotes: 3

Rakward
Rakward

Reputation: 1707

Perhaps you need mysql_real_escape_string() ?

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

Upvotes: 1

Related Questions