10

This is my code:

  $email= mysqli_real_escape_string($db_con,$_POST['email']);
  $psw= mysqli_real_escape_string($db_con,$_POST['psw']);

  $query = "INSERT INTO `users` (`email`,`psw`) VALUES ('".$email."','".$psw."')";

Could someone tell me if it is secure or if it is vulnerable to the SQL Injection attack or other SQL attacks ?

Gumbo
  • 620,600
  • 104
  • 758
  • 828
xRobot
  • 24,150
  • 62
  • 173
  • 291
  • 3
    possible duplicate of [SQL injection that gets around mysql\_real\_escape\_string()](http://stackoverflow.com/questions/5741187/sql-injection-that-gets-around-mysql-real-escape-string) – uri2x Sep 04 '15 at 07:55
  • 1
    I don't see how. mysqli_real_escape_string() is a completely different function than mysql_real_escape_string()... Maybe most people just didn't know that in 2015? – Ayelis Jul 19 '18 at 06:39

1 Answers1

13

Could someone tell me if it is secure or if it is vulnerable to the SQL Injection attack or other SQL attacks ?

No. As uri2x says, see SQL injection that gets around mysql_real_escape_string().

The best way to prevent SQL injection is to use prepared statements. They separate the data (your parameters) from the instructions (the SQL query string) and doesn't leave any room for the data to contaminate the structure of your query. Prepared statements solve one of the fundamental problems of application security.

For situation where you cannot use prepared statements (e.g. LIMIT), using a very strict whitelist for each specific purpose is the only way to guarantee security.

// This is a string literal whitelist
switch ($sortby) {
    case 'column_b':
    case 'col_c':
        // If it literally matches here, it's safe to use
        break;
    default:
        $sortby = 'rowid';
}

// Only numeric characters will pass through this part of the code thanks to type casting
$start = (int) $start;
$howmany = (int) $howmany;
if ($start < 0) {
    $start = 0;
}
if ($howmany < 1) {
    $howmany = 1;
}

// The actual query execution
$stmt = $db->prepare(
    "SELECT * FROM table WHERE col = ? ORDER BY {$sortby} ASC LIMIT {$start}, {$howmany}"
);
$stmt->execute(['value']);
$data = $stmt->fetchAll(PDO::FETCH_ASSOC);

I posit that the above code is immune to SQL injection, even in obscure edge cases. If you're using MySQL, make sure you turn emulated prepares off.

$db->setAttribute(\PDO::ATTR_EMULATE_PREPARES, false);
Scott Arciszewski
  • 32,288
  • 16
  • 86
  • 203
  • Saying "this is the best way" doesn't answer the question, "Is this enough?" – j_allen_morris Jul 07 '21 at 15:30
  • 1
    The answer is "No". The first sentence of my answer was "see this example" which proves it's not enough. – Scott Arciszewski Jul 14 '21 at 16:32
  • @ScottArciszewski - the citation you gave was for an answer about `mysql_real_escape_string()` which afaik is a different function than `mysqli_real_escape_string()`. Although your advice is not bad, I don't think this answer is correct. I've been building queries with user form input escaped with `mysqli_real_escape_string()` for over a decade and despite many thousands (millions?) of observed attempts at injection, not one has succeeded. If you believe it's not secure, I'd like to see a demonstration. – Eaten by a Grue Apr 07 '22 at 12:20