0

I have a contact form in my website rexhin.al where visitors can contact me. It has 3 fields only: name, phone number, and message.

This is my function for sanitizing input data which will be written in a database. I checked the php manual and a few questions here at stackoverflow and I came to this solution. Is this a safe way for sanitizing data? Are the functions in the correct order? Does the order really matter?

spl_autoload_register(function ($class) {
    include '../classes/' . $class . '.class.php';
});

/*
    beje qe nqs ekziston ip bej update vtm time dhe ++$count te ajo ip;
*/

$db = DB::get_instance();   

function sanatize($input) {
    $db = DB::get_instance();
    //mysqli real escape string for all vars.
    //preg_replace for whitespaces, tabs, new lines.
    //strip html tags.
    //convert html tags.
    //strip slashes.
    //htmlentities: htmlentities — Convert all applicable characters to HTML entities.
    //nuk duhet sepse kemi strip tags.

    //trim string
    $trimed_string = trim($input);
    //filter string using php FILTER_SANITIZE_STRING filter.
    $filtered_string = filter_var($trimed_string, FILTER_SANITIZE_STRING);
    //remove slashes
    $no_slash_string = stripslashes($filtered_string);
    //convert special characters to HTML entities
    $conv_string = htmlspecialchars($no_slash_string);
    //strip html tags
    $stripped_tags_string = strip_tags($conv_string);
    //replace whitespaces
    $filtered_string = preg_replace('#[\s]+#', ' ', $stripped_tags_string);
    $safe_string = $mysqli_escaped_string = $db->mysqli->real_escape_string($filtered_string);

    return $safe_string;
}

//send message
if ($_SERVER["REQUEST_METHOD"] == "POST") {
    if(isset($_POST["name"]) && isset($_POST["tel"]) && isset($_POST["message"])) {

        $name = sanatize($_POST["name"]);
        $tel = intval(sanatize($_POST["tel"]));

        //sepse intval ja heq zeron.
        $message = trim(sanatize($_POST["message"]));
        $time = time();

        //name validation.      
        //only letter and spaces.
        if(!preg_match('/^[a-zA-Z\s]+$/', $name)) {
            echo "name should contain only letters.";
        } else if(strlen($_POST["name"]) < 3) {
            echo "name should be three chars min.";
        } else if(!preg_match('/^[1-9][0-9]*$/', $tel)) {
            echo "your phone number should contain only numbers.";
        } else if(strlen($tel) != 9) {
            echo "your phone number must be 10 digits.";
        } else if(in_array(substr($tel, 0, 3), array(066, 067, 068, 069))) {
            echo "your phone number must begin with 066, 067, 068 or 069.";
        } else if(strlen($message) == 0) {
            echo "message should be 10 letters min.";
        } else {
            //insert into db.
            $query = "insert into `messages` (name, tel, message, time) VALUES ('$name', '$tel', '$message', '$time')";
            $db->mysqli->query($query);
            echo "sent";
        }
    }
}
BenMorel
  • 34,448
  • 50
  • 182
  • 322
M1X
  • 4,971
  • 10
  • 61
  • 123
  • `\s` match any white space character `[\r\n\t\f ]` – Braj Jul 20 '14 at 13:34
  • 1
    Sanitizing, like escaping, should be context-specific. What are you trying to constrain your input to? "Safe" for what use specifically? And why aren't just you using mysqli bound parameters? – mario Jul 20 '14 at 13:36
  • yes `\s` full-fill it. – Braj Jul 20 '14 at 13:37
  • Check my edited question @mario. I included my whole file. – M1X Jul 20 '14 at 13:41
  • The prior escaping then is kinda redundant if you validate the actual string formats afterwards anyway. The proper order for you sanitization function should have been `stripslashes`, `strip_tags`, `filter_var`, `spaces`, `trim`, `htmlspecialchars` and then `real_escape_string` btw. One "super" escaping function for everything is usually not a good idea. – mario Jul 20 '14 at 13:51
  • 2
    See also [How can I prevent SQL-injection in PHP?](http://stackoverflow.com/q/60174) – mario Jul 20 '14 at 13:53

1 Answers1

1

Relevant bits are these:

filter_var($trimed_string, FILTER_SANITIZE_STRING);
$db->mysqli->real_escape_string($filtered_string);

Though one could argue that prepared statements are easier to use than manual escaping (thus less likely that you miss a field) this should be enough from the input point of view. Even lack of filter_var() wouldn't be specially bad if you took proper care of SQL injection.

But you also perform lots of edits on user data that are fairly misguided:

stripslashes($filtered_string);
htmlspecialchars($no_slash_string);
strip_tags($conv_string);
preg_replace('#[\s\t\n]+#', ' ', $stripped_tags_string);

I wouldn't consider a contact form reliable if I wasn't able to recover the exact input data provided by the user and you're basically removing arbitrary characters and corrupting data.

I think the root problem are a couple of misconceptions:

  1. Data sanitisation as not an absolute. Whether a given piece of data is considered safe is entirely context sensitive. E.g., HTML tags are irrelevant in SQL.

  2. You don't have to remove special characters in order to make them safe. You just need to handle them properly.

Álvaro González
  • 142,137
  • 41
  • 261
  • 360