Ejercicio de fin de Semana: Evitar SQL Injection con PHP

A través de tweako (visto hace algunos minutos en menéame) llegué a un artículo -en inglés- que explica como protegerse de ataques de Inyección de SQL con PHP. En el mencionado artículo, en la última parte aparecen las siguientes dos porciones de código:

php:
<?php

# Ok, so I'm going to oversecure a query to the database that selects an article
# by using the given article ID.
# Here is the code.

//Database connection is present
//Make sure that the id is actually given

if (isset($_GET['id']))
{
        $id = $_GET['id'];
}
else
{
        die('Please provide an article ID');
}
 
//Make sure that its an integer
if (is_integer($id))
{
        die('Please enter a valid article ID');
}
 
//Validate that its in between the ranges 1 and 10,000
if ($id < 1 || $id > 10000)
{
        die('Please enter a valid artile ID');
}
 
//Construct the query
$SQL = "SELECT * FROM posts WHERE postID = '".$id."'";

echo $SQL; // Línea agregada

?>
php:
<?php

# This next one will validate a username before its entered into the database.

//Database connection is present
//Make sure that the id is actually given
if (isset($_GET['username']))
{
        $username = $_GET['username'];
}
else
{
        die('Please provide a username');
}
//Get the length of the username
$length = strlen($username);
//Validate the length
if ($length < 3 || $length > 20) // parte modificada
{
        die('Please enter a username between 3 and 20 characters long');
}
//Make sure that its safe to enter the database.
$username = mysql_real_escape_string($username);
//Construct the query
$SQL = "SELECT * FROM username WHERE username = '".$username."'";
//Show the username
echo 'Username: '.stripslashes($username);
//Send the query and close the connection to the database

?>

Los códigos mostrados ¿son correctos? si no es así, ¿qué errores tiene?.

9 thoughts on “Ejercicio de fin de Semana: Evitar SQL Injection con PHP”

  1. A partir de PHP 5 si, pero con esta entrada sólo intentaba mostrar que muchas veces existe código mal hecho :)

    Saludos

  2. Para el primero no vi errores mas que de que se deperdicia mucho codigo para hacer tan poca cosa, bien se puede hacer en 3 o 4 lineas de codigo eso... (talvez hasta dos).

    Para el segundo solo encontre que basarse en mysql_real_escape_string() como sabemos no estan seguro usarla ya que podemos(aunque depende de la version de MySQL ) aun asi podemos pasar el filtro, el problema seria el maximo de 20 largo pero yo creo que si se podra hacer algo con 20 caracteres.

    Aunque aun no veo como hacer el XSS que dices en el comentario que dejaste en el articulo original, seria bueno saber tu opinion...

    Saludos

  3. Para el primer ejemplo hay un error en la condición donde se comprueba que el parámetro id sea numérico, debería ser: if (!is_integer(...)) Por esa pequeña equivocación es posible alterar la consulta -y si, estoy de acuerdo en que hay demasiado código innecesario :)

    Para el segundo ejemplo, en condiciones normales no debería ser posible modificar la consulta, aunque como dices depende si la versión de MySQL es vulnerable o no (¿te refieres a los problemas con la codificación no?). Lo de hacer XSS lo dije en plan de fastidioso, porque en la última línea se escribe el nombre del usuario :P

  4. Alex una consulta, seria mejor colocar !is_numeric en vez de !is_integer ya que puede ser un valor real y no solamente entero, o bastaria con la primera opcion, y eso de hacerlo en 3 lineas, suena interesante aunque solamente veo que se pueden ahorrar if's.

    Y sobre el segundo ejemplo, es acaso la versiones anteriores a la 5x mas vulnerables que las actuales?

  5. MaXac, el uso de is_numeric o is_integer depende del tipo de datos que se haya definido para ese campo. El primer ejemplo, sin hacer ningún tipo de validación, se podría reducir en algo como:

    php:
    $SQL = "SELECT * FROM posts WHERE postID = ".(int)$_GET['id']; // o float, dependiendo el tipo de dato

    Sobre lo que mencionaba g3org3_x para el segundo ejemplo, me parece que se refería a un bug de MySQL que dependiendo de la codificación de la conexión que se realizaba con la BD, era posible sobrepasar la acción de mysql_real_escape_string, algo como esto debería funcionar en una versión vulnerable de MySQL:

    php:
    <?php

    $c = mysql_connect("localhost", "user", "pass");
    mysql_select_db("database", $c);

    // change our character set
    mysql_query("SET CHARACTER SET 'gbk'", $c);

    // create demo table
    mysql_query("CREATE TABLE users (
        username VARCHAR(32) PRIMARY KEY,
        password VARCHAR(32)
    ) CHARACTER SET 'GBK'"
    , $c);
    mysql_query("INSERT INTO users VALUES('foo','bar'), ('baz','test')", $c);

    // now the exploit code
    $_POST['username'] = chr(0xbf) . chr(0x27) . ' OR username = username /*';
    $_POST['password'] = 'anything';

    // Proper escaping, we should be safe, right?
    $user = mysql_real_escape_string($_POST['username'], $c);
    $passwd = mysql_real_escape_string($_POST['password'], $c);

    $sql = "SELECT * FROM  users WHERE  username = '{$user}' AND password = '{$passwd}'";
    $res = mysql_query($sql, $c);
    echo mysql_num_rows($res); // will print 2, indicating that we were able to fetch all records

    ?>

  6. Yo normalmente tengo algunas funciones como:

    php:
    function &#038;getPostVar($sVar, $sType, $mDefaultValue = NULL)
    {
       if (isset($_POST[$sVar])
       {
          switch($sType)
          {
              case 'int':
                 $_POST[$sVar] = (int) $_POST[$sVar];
                 break;

              /** y así por cada tipo **/
          }
          return $_POST[$sVar];
       }
       else
       {
          /* No vino nada vía HTTP-POST llamado $sVar */
          /* Devuelvo el valor por defecto */
          return $mDefaultValue;
       }
    }
     

    Esta función devuelve una referencia al valor de la variable que le estoy indicando en el primer parámetro que ha llegado vía POST. (se puede implementar para GET, COOKIES, etc.)

    El uso es bien fácil:

    php:
    $iNotId =&#038; getPostVar('not_id', 'int', NULL);
     

    De esta forma, el valor de $iNotId ya se de entrada que es entero y lo puede utilizar en cualquier lugar estando 100% seguro, sin necesidad de estar aplicando filtros en medio del armado de las instrucciones SQL que entorpecen el código para mi gusto.

    Hago que devuelva una referencia al elemento del array $_POST simplemente para ahorrar memoria y no repetir datos inútilmente en el script.

  7. y se puede hacer sql injection en un buscador?? me gustaria saber que vulnerabilidad es comun, y seria bueno mostrarlo para que todos arreglen sus buscadores :D

Comments are closed.