Jump to content

help me to secure my php mysql form


linux1880

Recommended Posts

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

Link to comment
Share on other sites

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

 

There are two security issues I see right off the bat.

 

The first one is that you're embedding variables directly into the SQL string.  You should instead prepare the statement, bind the variables, and then execute the statement.  This will protect you from the dreaded SQL injection.  Check out the examples here:  http://www.php.net/manual/en/mysqli.prepare.php.  If you're feeling lazy, you can at least run the fields through mysql_real_escape_string() before embedding it:

 

$fullname = mysql_real_escape_string($fullname);
$photo = mysql_real_escape_string($photo);
$sql = "INSERT INTO members(fullname,photo) VALUES('$fullname', '$photo');

 

The second is that you are not filtering the uploaded file.  A user could upload a PHP file to your server, go directly to it, and take over your site.  Validate the mimetype of the file, and since it is a photo, you can throw it through some image processing functions before moving it to its final directory.  Here are some great examples:  http://www.php.net/manual/en/features.file-upload.php

 

You can check to see if a file already exists by using file_exists()

Link to comment
Share on other sites

Hi there,

 

There are many exploitable parts to your PHP script.

 

Below is a commented and fixed version of your script:

 

//Always sanitize every user input!

$fullname = mysql_real_escape_string(addslashes($_POST['fullname']));

    $uploaddir = './uploads/';

    //Possible SQL Injection Exploit (if the file name is something like ' or 1=1-- 
    //Also possible Full Path Disclosure exploit

    $uploadfile = $uploaddir. basename(mysql_real_escape_string(stripslashes($_FILES['photo']['name'])));

    $upload_filename = basename($_FILES['photo']['name']);
    move_uploaded_file($_FILES['photo']['tmp_name'], $uploadfile);
    $photo = mysql_real_escape_string($upload_filename);


//ALWAYS ALWAYS Sanitize SQL DATA!
$sql = "INSERT INTO members(fullname,photo) VALUES('{$fullname}', '{$photo}');
$stmt = $link->query($sql) or die($link->error);
    $stmt->close;

 

 

 

This script should be fine to use now, it should be sanitized. I cannot Guarentee 100% as I haven't seen your $stmt class or your $link->query() function.

 

Also try sanitizing the File types, use only JPEG, JPG, and PNG for example.

 

Hope this helps,

Best Regards,

Mantyy

Link to comment
Share on other sites

You need to implement a check similar to:

 

$valid_mimes = array(
    'image/png',
    'image/gif',
    'image/jpeg',
    // etc..
);

if (!in_array($_FILES['photo']['type'], $valid_mimes))
{
    // handle error here
}

Link to comment
Share on other sites

Thanks adam, what is you best suggestion for making a file name unique ?

 

Why dont you just take the original name and add some random numbers to it.  that would make it unique enough.

like:

<?php
$filename = "fatmonkeys.jpg";
$final_name = rand(10,10000).$filename;
echo 'filename: '.$filename.'<br />';
echo 'final_name: '.$final_name.'<br />';
//will print out something like:
//filename: fatmonkeys.jpg
//final_name: 3543fatmonkeys.jpg
?>

Link to comment
Share on other sites

This thread is more than a year old. Please don't revive it unless you have something important to add.

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.