mysql filter on exact match and how to make it more safe

Question

I recently started programming in php and made some nice progress (if I say it myself). Now I stumbled upon some problems.

What I have is some if statements with a mysql query inside it.

if(empty($_POST['Email'])){
    $error_Email = "No email";
    $errors++;
}elseif(!preg_match("/([\w\-]+\@[\w\-]+\.[\w\-]+)/", $_POST['Email'])){
    $error_Email = "Invalid email";
    $errors++;
}elseif ($mysqli->query("SELECT * FROM Members WHERE Email LIKE '".$_POST['Email']."'")->num_rows == 0){
    $error_Email = "[TEST] NOT IN DB";
}else{
    $error_Email = "[TEST] IN DB";
}

So I have a couple problems with my code. Lets say that me@this.com is in the database. I only want to see [TEST] IN DB when I type me@this.com exactly, not if me@, not if this.com, ... only when I type in me@this.com.

My 2nd problem is that this piece of code can be manipulated because I put the search criteria in the query directly.

So is there a safer way of achieving what I want above with the query returning a row only if the match is exact.

For exact matches, you use `=` in SQL; and using LIKE is not particularly meaningful unless your data uses wildcard characters — Mark Baker, Sep 27 '14 at 15:56
To make your database queries more secure, you're using MySQLi so you can use [prepared statements with bind variables](http://php.net/manual/en/mysqli.quickstart.prepared-statements.php) — Mark Baker, Sep 27 '14 at 15:57
@Sumurai8, this is not a duplicate since there is a 2nd part in my question. — Krowi, Sep 27 '14 at 16:01
Questions are supposed to be about one problem, not an aggregate of different questions. I choose this as the duplicate, because I perceive this as the dominant problem of the two. The dupe contains all the information you want about the security aspect of the question. The answer to the second question is as simple as "use =", and thus I see that as a secondary, less important, question. — Sumurai8, Sep 27 '14 at 16:12

Nick_Core · Accepted Answer · 2014-09-27T16:54:09.500

1

I think you can make this look a lot cleaner with something like this

$email = $_POST['Email'];
$email_status = validateEmail($email);
function validateEmail($email){
  if(empty($email)){
    return array("status" => 0, "error" => "Empty email");
  }
  if(!preg_match("/([\w\-]+\@[\w\-]+\.[\w\-]+)/", $email){
    return array("status" => 0, "error" => "Invalid email");
  }else{
    $stmt = $mysqli->prepare('SELECT * FROM Members WHERE Email = ?');
    $stmt->bind_param('s', $email);
    $stmt->execute();
    $result = $stmt->get_result();
    if(result->num_rows == 0){
      return array("status" => 0, "error" => "valid email, but no matches!");
    }else{
      return array("status" => 1, "error" => "");
    }
  }
}

later in the view you use

<input type="text" name="email"/>
<?php if($email_status["status"] === 0){
  echo $email_status["error"];
} ?>

edited Sep 27 '14 at 16:54

answered Sep 27 '14 at 16:06

Nick_Core

136
7

I think you should add a condition to check for email empty and do not use to conditions but them in one If else statement. – Abdessabour Mtk Sep 27 '14 at 16:08
Thanks for you answer. Is this code fully functional without javascript? I also added my answer I came up with, could you have a look at that and maybe optimize? – Krowi Sep 27 '14 at 16:14
I'm sure your solution works fine, but in serions large apps you will have a lot of validation rules, and it can get messy, so i would wrap them all in big functions like validateThat($blabla); validateCreditCard($card_number); which return the error if validation fails. – Nick_Core Sep 27 '14 at 16:24
It's indeed a good idea to put these in functions and maybe in a different file so I can implement it in different pages. If you could update your answer with the security to prevent sql injection like in my answer then I can mark yours as the final answer for my question. – Krowi Sep 27 '14 at 16:34

score 1 · Answer 2 · answered Sep 27 '14 at 16:12

I came up with a solution myself. I asked this question way too early but don't want to remove it for the sake of helping other people who might find this.

In the code below I only check the database if the email is not empty and a valid one to reduce requests. I think this is a good solution to my question. Suggestions are of course still welcome.

if(empty($_POST['Email'])){
    $error_Email = "No email";
    $errors++;
}elseif(!preg_match("/([\w\-]+\@[\w\-]+\.[\w\-]+)/", $_POST['Email'])){
    $error_Email = "Invalid email";
    $errors++;
}else{
    $stmt = $mysqli->prepare('SELECT * FROM Members WHERE Email = ?');
    $stmt->bind_param('s', $_POST['Email']);
    $stmt->execute();
    $result = $stmt->get_result();

    if($result->num_rows == 0){
    $error_Email = "NOT IN DB";
    }else{
    $error_Email = "IN DB";
    }
}

+1 for starting to using bind variables – Mark Baker Sep 27 '14 at 16:29 — Mark Baker, Sep 27 '14 at 16:29

mysql filter on exact match and how to make it more safe

2 Answers2