0

I've recently created a PHP registration script for a website that a friend and I are working on. I've read up on the dangers of MySQL injection, and I was wondering if my current code is protected from such attacks, or if additional steps need to be taken.

The relevant part of the code is as follows

  $email = $_POST['email'];
  $username = $_POST['username'];

  $username = strip_tags(trim($username));
  $email = strip_tags(trim($email));

  //Sanitation code I found somewhere..
  function mres($var){
    $conn = get_my_db();

    if (get_magic_quotes_gpc()){
        $var = stripslashes(trim($var));
    }
    return $conn->real_escape_string(trim($var));
  }

  //Sanitize username
  $sanUsername = mres($username);

  //Make SQL query
  $sql = "SELECT username FROM accounts WHERE username = '$sanUsername' ";
  $result = $conn->query($sql);

Thanks in advance,

Shawn

EDIT:

I've updated the SQL query to the following to make use of Prepared Statements

  //Prepare statement
  $prep = $conn->prepare("SELECT username FROM accounts WHERE username = ?");
  $prep->bind_param("s", $sanUsername);
  $prep->execute();
  $result = $prep->get_result();

closed as off-topic by symcbean, Xander, Mark, TildalWave, Steve Dec 25 '14 at 5:15

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Questions asking us to break the security of a specific system for you are off-topic unless they demonstrate an understanding of the concepts involved and clearly identify a specific problem." – Xander, Mark, TildalWave, Steve
If this question can be reworded to fit the rules in the help center, please edit the question.

  • 1
    Should be asked on codereview.stackexchange.com – symcbean Dec 24 '14 at 12:55
5

You're using the wrong approach to prevent SQL injection. Sanitizing inputs can help provide security, but by far the best way to avoid SQL injection is to remove the thing that allowed it in the first place: mixing code and (user-supplied) data in the same string, such that the database doesn't know what was supposed to be code and what was supposed to be data.

There is no need to do this in SQL anymore, and you want to look into prepared statements. The way those work is that you first send the database everything but the actual data; the database compiles the query, and only then do you supply the actual data of the query. What this means is that the user-supplied data is placed by the server into specific slots in the query, and cannot change what the server is doing.

To be more explicit, you send the server something like SELECT username FROM accounts WHERE username = ?. The server processes this query (the titular prepared), turning it into a function that's just waiting for a value for the username. You then bind the specific username to the statement, and execute it. The key is that the server already knows exactly what it should do before it ever sees user data; nothing a malicious user sends can change what the database is going to do (i.e. find rows with the given username and return their username).

Sanitization of database inputs is fundamentally an attempt to mitigate an attack vector by blocking inputs that exploit it. Prepared statements actually remove this attack vector, because they eliminate the conditions that allowed it to exist.

Details of how to use prepared statements vary by language, framework, and database server; look up PHP/MySQL documentation for the actual details.

  • Thanks for the helpful response! I've updated my code above to make use of Prepared Statements. – Shawn Anastasio Dec 25 '14 at 7:20

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