1

I have to receive a variable from URL (GET) and use it. It's just a number (from 0 to 400) and with that value I do a query.

To make sure no one is doing something wrong (SQL Injection or something else, I'm not an expert on security), I used: mysqli_real_escape_string() and also used an IF: if($id > 0 and $id < 400)

Do you think it's enough? the entire code is below. Thanks!

if($_SERVER["REQUEST_METHOD"] == "GET") {
      // id enviado desde GET

    $id = mysqli_real_escape_string($db,$_GET['id']); // $db es la conexión MySQL
    if($id > 0 and $id < 400) {   
        $sql = "SELECT url FROM urls WHERE id = '$id'"; // Traemos la primera URL del id correspondiente
        $result = mysqli_query($db,$sql); 
        echo "Cantidad de rows devueltas x la consulta SQL:" ;
        echo mysqli_num_rows($result);
        echo "<br>";

        while ($row = mysqli_fetch_array($result)) {
                echo $row[0];
                echo "<br>";
        }
    }
   }
Max
  • 85
  • 1
  • 10
  • 6
    Make it safer by using a prepared statement with bind variables - [Here's a problem demonstration with your code](https://3v4l.org/c15DS) – Mark Baker Jun 01 '16 at 15:50
  • For id you don't need to escape it, check it with is_numeric() instead. – Viktor Koncsek Jun 01 '16 at 15:53
  • 4
    **No**, this is **not safe**. *Changing* the user input before *executing it as code* will never be safer than *not executing it as code in the first place*. Some examples: http://stackoverflow.com/questions/5741187/sql-injection-that-gets-around-mysql-real-escape-string Treating user input as *data* instead of *code* is how you protect against SQL injection. Use query parameters. – David Jun 01 '16 at 15:54
  • If I use is_numeric() is it still unsafe? or if I limit the characters of the variable to max of 3? how could someone possibly hack me doing that? – Max Jun 01 '16 at 16:01
  • If you alter the code in the future you may forget you put that safeguard in place. If you use parameterized queries regardless of what you do in the code your query will be safe. – chris85 Jun 01 '16 at 16:18
  • Ok thanks, I will research parametrized queries. Thanks again for your help! – Max Jun 01 '16 at 17:17

1 Answers1

0

mysqli_real_escape_string is good but prepared statement is better, because you sometimes want to insert special characters to your db:

/* create a prepared statement */
if ($stmt = $mysqli->prepare("SELECT District FROM City WHERE Name=?")) {

    /* bind parameters for markers */
    $stmt->bind_param("s", $city);

    /* execute query */
    $stmt->execute();

    /* bind result variables */
    $stmt->bind_result($district);

    /* fetch value */
    $stmt->fetch();

    printf("%s is in district %s\n", $city, $district);

    /* close statement */
    $stmt->close();
}

this of course is sql injection save

njank
  • 319
  • 3
  • 12