Impedir la inyección código en mi pagina

[diego007]

Hola amigos, descargue el web securify scaner, y el sistema me muestra algunos errores, pero entre los mas importantes al momento es la inyeccion de codigo Sql que se puede hacer en estas direcciones

Código PHP:

  lc.php?articulo=1&pagina=1&ordenar=%22'%3CXPLZ6%3E HTTP/1.1
lc.php?articulo=1&ordenar=%20fecha%20DESC%20&pagina=%22'%3CXPLZ6%3E HTTP/1.1 
   

mi código es el siguiente:

Código PHP:

  $registros = 30;
$pagina = stripslashes(mysql_real_escape_string(htmlspecialchars(@$_GET['pagina'])));
$id = stripslashes(mysql_real_escape_string(htmlspecialchars(@$_GET['articulo'])));
$estado = stripslashes(mysql_real_escape_string(htmlspecialchars(@$_GET['estado'])));
$ordenar = stripslashes(mysql_real_escape_string(htmlspecialchars(@$_GET['ordenar'])));
$sentencia = "";
$titulo = "Esta buscando ";
 
if($ordenar == ""){
    $ordenar = " fecha DESC ";
}else{
    if($ordenar == "desendente"){
        $ordenar = " precio DESC ";
    }else{
        if($ordenar == "ascendente"){
            $ordenar = " precio ASC ";
        }else{
            if($ordenar == "nuevo"){
                $ordenar = " fecha DESC ";
            }else{
                if($ordenar == "antiguo"){
                    $ordenar = " fecha ASC ";
                }
            }
        }
    }
}
 
if($estado != ""){
    $estado = mysql_real_escape_string($estado);
    $sentencia = $sentencia." condicion ='$estado' AND ";
    $titulo = $titulo." en condicion \"".$estado."\"";
}
 
 
$pagina = stripslashes(mysql_real_escape_string(htmlspecialchars(@$_GET["pagina"])));
if (@!$pagina) 
{ 
    $inicio = 0; 
    $pagina = 1; 
} else { 
    $inicio = ($pagina - 1) * $registros; 
}
 
    
// ------------------- Filtros -------------------------------
 
echo "<div id=\"filtros\">";
echo "<table align=\"center\" style=\"width: 1000px; height:20px;\" ><tr>";
 
echo "<td><a href=\"lc.php?";
    if (@$id != ""){ echo "articulo=".$id."&";}
    if (@$estado != ""){ echo "estado=".$estado."&";}
    if (@$ordenar != ""){ echo "ordenar=".$ordenar."&";}
    if (@$pagina != "") { echo "pagina=".$pagina."&";} 
echo " \">Listado</a> | <a href=\"lc_1.php?";
    if (@$id != ""){ echo "articulo=".$id."&";}
    if (@$estado != ""){ echo "estado=".$estado."&";}
    if (@$ordenar != ""){ echo "ordenar=".$ordenar."&";}
    if (@$pagina != "") { echo "pagina=".$pagina."&";} 
echo " \">Galería </a></td>";
 
echo "<form name=\"filtros\" id=\"filtros\">";
if(@$_GET["estado"] == ""){
    echo "<td>Condiciòn: </td><td>";
    echo "<select name=\"estado\" id=\"estado\" onChange=\"url2(document.getElementById('estado').value)\">";
    echo "<option value=\"\"></option>";
    echo "<option value=\"Nuevo\" >Nuevo</option>";
    echo "<option value=\"Usado\" >Usado</option>";
    echo "</select></td>";
}else{
echo "<td><a href=\"lc.php?";
    if (@$id != ""){ echo "articulo=".$id."&";}
    if (@$ordenar != ""){ echo "ordenar=".$ordenar."&";}
    if (@$pagina != "") { echo "pagina=".$pagina."&";} 
    echo "\">Estado [".ucfirst($_GET['estado'])."]<strong>X</strong></a></td>";
}
echo "</form>";
 
echo "<td><a href=\"lc.php?";
    if (@$id != ""){ echo "articulo=".$id."&";}
    if (@$estado != ""){ echo "estado=".$estado."&";}
    if (@$pagina != "") { echo "pagina=".$pagina."&";} 
echo "ordenar=desendente\" >[Mayor precio]</a></td>";
 
echo "<td><a href=\"lc.php?";
    if (@$id != ""){ echo "articulo=".$id."&";}
    if (@$estado != ""){ echo "estado=".$estado."&";}
    if (@$pagina != "") { echo "pagina=".$pagina."&";} 
echo "ordenar=ascendente\" >[Menor precio]</a></td>";
 
echo "<td><a href=\"lc.php?";
    if (@$id != ""){ echo "articulo=".$id."&";}
    if (@$estado != ""){ echo "estado=".$estado."&";}
    if (@$pagina != "") { echo "pagina=".$pagina."&";} 
echo "ordenar=nuevo\" >[Más nuevo]</a></td>";
 
echo "<td><a href=\"lc.php?";
    if (@$id != ""){ echo "articulo=".$id."&";}
    if (@$estado != ""){ echo "estado=".$estado."&";}
    if (@$pagina != "") { echo "pagina=".$pagina."&";} 
echo "ordenar=antiguo\" >[Más antiguo]</a></td>";
 
echo "</tr>";
echo "<tr><td COLSPAN=9><br><HR width=100% align=\"center\" style=\"border: 1px solid #DAE4EF;\"><br></td></tr>";
echo "</table>";
echo "</div>";
 
 
 
$query = sprintf("SELECT * FROM articulo WHERE id= '%s' AND vendido='0' AND suspender='0' AND valorar='0' ORDER BY fecha DESC LIMIT 0 , 1", mysql_real_escape_string($id));
$result = mysql_query($query, $con); 
   

Que me recomiendan para hacerlo mas seguro al sitio, please

[gildus]

Holas,

Una opcion de las muchas que podria haber me imagino, es que en vez de enviar la palabra o valor de la variable "nuevo", "ascendente", "desendente", simplemente podrias enviar solo numeros o boleanos como true o false o sino 1 o 2 o 3, etc.

Si fuese numeros en el momento de recibirlos por GET o POST, podrias castearlos a que sean enteros o numeros o si fuesen boleanos que sean solo true o false y no aceptar ningun otro valor o cadena.

Te recomiento que siempre valides la cantidad de datos que recibas, que pasaria si solo recibes dos o una sola variable GET ?, habria error? o warning?.

Es bueno que uses mysql_real_escape_string, pero mejor aun te recomendaria usar el PDO. Haz intentado usar el mysqli ?, en especial en este caso mysqli_prepare:

http://php.net/manual/en/mysqli.prepare.php

Saludos

[diego007]

Gracias gildus, creo que solucione el problema usando lo siguiente:

Código PHP:

  if($ordenar == ""){
    $ordenar = " fecha DESC ";
}else{
    if($ordenar == "desendente"){
        $ordenar = " precio DESC ";
    }else{
        if($ordenar == "ascendente"){
            $ordenar = " precio ASC ";
        }else{
            if($ordenar == "nuevo"){
                $ordenar = " fecha DESC ";
            }else{
                if($ordenar == "antiguo"){
                    $ordenar = " fecha ASC ";
                }else{
                    $ordenar = " fecha DESC ";
                } 
   

y

Código PHP:

  $pagina = stripslashes(mysql_real_escape_string(htmlspecialchars(@$_GET["pagina"])));
$ejemplo = strlen($pagina);
if ($ejemplo > 3){
    $pagina = 1;
} 
   

ingrese de nuevo las direcciones y ya no me da ningún problema, el único problema sera cuando la pagina tenga mas de 1000 pies de pagina.

En este momento tengo un nuevo problema, el Websecurify scanner, me dice que otro problema que tenfo es "Open Redirect", esta es la explicación que me da el programa

An Open Redirect is a type of a vulnerability where a parameter taken from the user is used as the target of a HTTP redirect without any validation. This vulnerability is commonly used in phishing attacks. The purpose of the attack is to get the user to visit a malicious site without realizing it.

solution: Ensure that the application can only redirect to a whitelist of approved applications.

no tengo ni idea de como solucionarlo, les adjunto la direccion y el codigo para ver si me pueden dar una mano

index3.php?articulo=http%3A%2F%2FXPLZ6 HTTP/1.1

Código PHP:

  if ($_POST) {
 
    $preguntante = stripslashes(mysql_real_escape_string(htmlspecialchars($_SESSION['email'])));
    $articulo = stripslashes(mysql_real_escape_string(htmlspecialchars($_POST['articulo'])));
    $valorar = 0;
    $eliminar = 0;
    $hora = date('Y-m-d H:i:s', time() -10);
    // conocer la IP
    if(isset( $_SERVER ['HTTP_X_FORWARDED_FOR'] ) )
    {
        $ip = $_SERVER ['HTTP_X_FORWARDED_FOR'];
    }else{
        if( isset( $_SERVER ['HTTP_VIA'] ) ){
            $ip = $_SERVER ['HTTP_VIA'];
        }else{
            if( isset( $_SERVER ['REMOTE_ADDR'] ) ){
                $ip = $_SERVER ['REMOTE_ADDR'];
            }
        }
    }
    
    $query=sprintf("SELECT (id2) FROM preguntas ORDER BY id2 DESC LIMIT 1");
    $result = mysql_query($query, $con); 
    $id2 = 0;
    while($row = mysql_fetch_array($result))
    {
        $id2 = $row['id2'];
    }
    $id = md5(md5($id2 + 1));
    
    $query = sprintf("SELECT * FROM articulo WHERE id= '%s' AND vendido='0' AND suspender='0' ORDER BY fecha DESC LIMIT 0,1", mysql_real_escape_string($articulo));
    $result = mysql_query($query, $con); 
    if(mysql_num_rows($result) > 0){
        while($row=mysql_fetch_array($result))
        {
            $usuario = $row['1'];
        }    
    }
    
    $query = sprintf("SELECT * FROM articulo WHERE id= '%s' AND vendido='0' AND suspender='0' ", mysql_real_escape_string($articulo));
    $result = mysql_query($query, $con); 
    if(mysql_num_rows($result) > 0){
        $query = sprintf("SELECT * FROM preguntas WHERE articulo= '%s' AND preguntante= '%s'  ORDER BY fecha DESC LIMIT 0,1 ", mysql_real_escape_string($articulo), mysql_real_escape_string($preguntante));
        $result = mysql_query($query, $con);
        if(mysql_num_rows($result) > 0){
            while($row=mysql_fetch_array($result))
            {
                $fecha = $row['5'];
            }
            if($fecha < $hora){    
                //if(trim($_POST["pregunta"]) != "" && $usuario != "" && $articulo != "" )
                if(trim($_POST["pregunta"]) != "" && $usuario != "" && $articulo != "" && $usuario != $preguntante )
                {
// aqui es donde se ingresan los datos 
   

[diego007]

se me paso comentarles que el tipo de artículos tienen codificacion md5, y son unicos, y son como este ejemplo: 4fc8f8e36367a2f5ff42b38a489d4b3d

[gildus]

Me imagino que si tratas de redireccionar luego de un momento de logear algo como:

index3.php?articulo=htt://www.dominio.com/contenido.html&.....

Si haces eso, creo que estaria mal y creo que a eso se refiere el scanner, no redirecciones por URL es decir enviando valores URL al get ni al post usa header().

Saludos

[diego007]

este index3.php lo que hace es ingresar preguntas. Para lo cual se necesita estas logeado.
Primero lo que cace el sistema antes de mostrarme el index3.php (formulario de ingreso de la pregunta) es abrir otro archivo PHP (comprueba.php) el cuan detecta si estas logeado o no. silo estas te envia al index3.php, si no lo estas, te envia a la pagina de login. Para redireccionar no uso header(), uso javascript

Código PHP:

  echo "<script language='JavaScript'>";
         echo "location = 'outdatos/index3.php?articulo=".$_POST['articulo']."'";
         echo "</script>"; 
   

El archivo Login.php envía los datos por medio de POST

[gildus]

Usa header del PHP, te recomiendo eso.

Luego el codigo que muestras que imprime un javascript, es incorrecto como redireccionas, no es location es location.href, y los datos no se envias por POST sino lo estas enviando por GET, una cosa es recibirlos por POST y otro es enviarlos por GET.

Saludos

[diego007]

perdón, me confundí en el mensaje anterior. el ingreso de mensajes es de la siguiente forma:
el index3.php tiene un if en la parte superio, el cual detecta la cesión, si no a iniciado cesion, es redirecionado al login.php, este archivo login.php tiene un formulario que envia los datos por POST al archivo comprueba.php, este archivo recibe los datos de usuario, y los datos de la pagina donde se encuentra para que después de logeado, el usuario regrese automáticamente a la pagina donde se encontraba.
Tienes toda la razon debo corregir el error de javascrip. Uso Java script ya que heder solo puede ser usado en la parte superior del código, y en mi caso, el redirecinamiento se encuentra en la mitad del código, justo despues de la parte donde confirmar el el nombre de usuario y el password.

Código PHP:

   $query = sprintf("SELECT usuarios.nick,
                             usuarios.nombre,
                              usuarios.apellido, 
                             usuarios.email,
                             usuarios.provincia,
                             usuarios.ciudad
                       FROM usuarios WHERE usuarios.email='%s' && usuarios.password = '%s'",  // Ahora
               mysql_real_escape_string($login),mysql_real_escape_string($pass));       
      $result=mysql_query($query,$con);
      // if(mysql_num_rows($result)==0){ // antes
      if(mysql_num_rows($result)){ // nos devuelve 1 si encontro el usuario y el password
      
        $array=mysql_fetch_array($result);
         //  if($array["password"]==crypt($pass,"semilla") ){ // Antes
          /* Comprobamos que el password encriptado en la BD coincide con el password que nos han dado al encriptarlo. Recuerda usar semilla para encriptar los dos passwords. */
         $_SESSION["login"]=$array["nick"];
         $_SESSION["nombre"]=$array["nombre"];
         $_SESSION["apellido"]=$array["apellido"];
         $_SESSION["email"]=$array["email"]; // Agrgado Nuevo
         $_SESSION["provincia"]=$array["provincia"];
         $_SESSION["ciudad"]=$array["ciudad"];
         //header("Location:user.php");
         echo "<script languaje='javascript' type='text/javascript'>window.close();</script>";
         //header("location: outdatos/index2.php?codigo=".$_POST['articulo']);
         echo "<script language='JavaScript'>";
         echo "location = 'outdatos/index2.php?codigo=".$_POST['articulo']."'";
         echo "</script>"; 
   

[diego007]

no seria mejor poner un if después de recibir el numero del articulo? por ejemplo: hacer que cuente los caracteres de la cadena recibida por GET o POST, y si hay mas de 32 o menos de 32, entonces que no muestre la ventana de login sino que sea redirecionado al index.php para que no muestre nada, creen que de resultado esta idea?

Uso el numero de 32 ya que cada registro de la base de datos es encriptado por md5, y nunca va a existir un valor superior o inferior a 32

[gildus]

header(), puede ser usado sin problemas, al inicio o despues o en alguna parte de tu codigo, es alli donde el scanner te menciona la redireccion, por eso trata de usar el header para redireccionar.

Saludos