-1
\$\begingroup\$

Initializing variables:

<? php
$host = 'localhost';
$user = 'root';
$pword = '';
$dbname = 'mydb';
$tablename = 'userdata';
$con = mysqli_connect($host, $user, $pword); 

Code for function to clean data to be sent to database (where $variables is an array which has data which is trimmed, strip_tags and htmlspecialchars) assuming that we are connected to the database:

function cleandbvalues($variables) {
$firstname = mysqli_real_escape_string($con, $variables['firstname']);
$lastname = mysqli_real_escape_string($con, $variables['lastname']);
$username = mysqli_real_escape_string($con, $variables['username']);
$password = mysqli_real_escape_string($con, $variables['password']);
$gender = mysqli_real_escape_string($con, $variables['gender']);
$course = $variables['course'];
$string = implode(",",$course);
$course = mysqli_real_escape_string($con,$string);
$comments = mysqli_real_escape_string($con, $variables['comments']);
}

(Note: The following code is for variables array)

function clean($data) {
$data = strip_tags(htmlspecialchars(stripslashes(trim($data)))); 
return $data;
}

function initialize(){
$var = array();
$var['firstname'] = isset($_POST['firstname']) ? clean($_POST['firstname']) : '';
$var['lastname'] = isset($_POST['lastname']) ? clean($_POST['lastname']) : '';
$var['username'] = isset($_POST['username']) ? clean($_POST['username']) : '';
$var['password'] = isset($_POST['password']) ? $_POST['password'] : '';
if(!empty($_POST['gender'])) {                
$var['gender'] = $_POST['gender'];
}  else {
    $var['gender'] = '';
}
//var_dump($_POST[gender]);
if(!empty($_POST['course'])) {               
$var['course'] = $_POST['course'];
} else {
    $var['course'] = '';
}
$var['comments'] = isset($_POST['comments']) ? clean($_POST['comments']) : '';
  return $var;
}

$variables = initialize();

Code for function to insert above cleaned data into table:

function insertvalues($firstname,$lastname,$username,$password,$gender,$course,$comments) {
    $sql = "INSERT INTO userdata 
 (uid,firstname,lastname,username,password,gender,course,comments)
 VALUES ('','$firstname','$lastname','$username','$password','$gender','$string','$comments')";
$con = mysqli_connect($host, $user, $pword); 
if (!mysqli_query($con,$sql)) {
  die('Error: ' . mysqli_error($con));
}
echo "1 record added";
}


mysqli_close($con);

?>

Are these the correct functions for cleaning data to be sent to database and inserting the data in database in PHP? I didn't want to use just plain code but wanted to use code inside functions for re-usability,so is it right?

\$\endgroup\$
  • 1
    \$\begingroup\$ What do you mean by "is this correct"? Have you tested it to make sure it does what it should? We can only review this code if it works as intended. \$\endgroup\$ – Jamal Sep 14 '14 at 5:27
2
\$\begingroup\$

I think that your code is a bit unclear as to how it actually works (what function is executed when), but I'll try my best to answer anyways.

Are these the correct functions for cleaning data to be sent to database and inserting the data in database in php?

No, they are not.

Preventing SQL Injection: General Overview

  • mysqli_real_escape_string is meant to be used to escape user input before inserting it into a database (to somewhat sometimes prevent SQL injection).
  • htmlspecialchars is meant to escape user input before outputting it to the user (to prevent XSS attacks in most cases).

This means that htmlspecialchars shouldn't be used for escaping database input at all.

So is mysqli_real_escape_string the right solution? No, because it is generally not recommended to rely on input escaping. Here is what the owasp says about escaping input:

[input escaping] is frail compared to using parameterized queries and we cannot guarantee it will prevent all SQL Injection in all situations. This technique should only be used, with caution, to retrofit legacy code in a cost effective way. Applications built from scratch, or applications requiring low risk tolerance should be built or re-written using parameterized queries.

This isn't just a theoretical concern either.

So you should use prepared statements instead. [Not relevant for your code, but in general: you should use prepared statements for all user input, as well as all values that you retrieved from the database which are based on user input (to prevent second order SQL injection)].

Other

  • in PHP, arguments are passed by value so your cleandbvalues function doesn't do anything.
  • please use proper formating (indentation, spaces, etc).
  • follow general naming conventions (cleandbvalues should be cleanDbValues or clean_db_values, same for insertvalues).
\$\endgroup\$

Not the answer you're looking for? Browse other questions tagged or ask your own question.