¿Existen vulnerabilidades de seguridad en este código PHP?

Question

Acabo de obtener un sitio para administrar, pero no estoy muy seguro sobre el código que escribió el chico anterior. Estoy pegando el siguiente procedimiento de inicio de sesión, ¿podría echar un vistazo y decirme si hay vulnerabilidades de seguridad? A primera vista, parece que se puede acceder a través de inyección SQL o manipulación de cookies y el parámetro? M =.

---

 

define ('CURRENT_TIME', time());// Current time. 
define ('ONLINE_TIME_MIN', (CURRENT_TIME - BOTNET_TIMEOUT));// Minimum time for the status of "Online". 
define ('DEFAULT_LANGUAGE', 'en');// Default language. 
define ('THEME_PATH', 'theme');// folder for the theme. 

// HTTP requests. 
define ('QUERY_SCRIPT', basename ($ _SERVER [ 'PHP_SELF'])); 
define ('QUERY_SCRIPT_HTML', QUERY_SCRIPT); 
define ('QUERY_VAR_MODULE', 'm');// variable contains the current module. 
define ('QUERY_STRING_BLANK', QUERY_SCRIPT. '? m =');// An empty query string. 
define ('QUERY_STRING_BLANK_HTML', QUERY_SCRIPT_HTML. '? m =');// Empty query string in HTML. 
define ('CP_HTTP_ROOT', str_replace ('\ \', '/', (! empty ($ _SERVER [ 'SCRIPT_NAME'])? dirname ($ _SERVER [ 'SCRIPT_NAME']):'/')));// root of CP. 

// The session cookie. 
define ('COOKIE_USER', 'p');// Username in the cookies. 
define ('COOKIE_PASS', 'u');// user password in the cookies. 
define ('COOKIE_LIVETIME', CURRENT_TIME + 2592000)// Lifetime cookies. 
define ('COOKIE_SESSION', 'ref');// variable to store the session. 
define ('SESSION_LIVETIME', CURRENT_TIME + 1300)// Lifetime of the session. 

////////////////////////////////////////////////// ///////////////////////////// 
// Initialize. 
////////////////////////////////////////////////// ///////////////////////////// 

// Connect to the database. 
if (! ConnectToDB()) die (mysql_error_ex()); 

// Connecting topic. 
require_once (THEME_PATH. '/ index.php'); 

// Manage login. 
if (! empty ($ _GET [QUERY_VAR_MODULE])) 
( 
// Login form. 
    if (strcmp ($ _GET [QUERY_VAR_MODULE], 'login') === 0) 
    ( 
    UnlockSessionAndDestroyAllCokies(); 

    if (isset ($ _POST [ 'user']) & & isset ($ _POST [ 'pass'])) 
    ( 
     $ user = $ _POST [ 'user']; 
     $ pass = md5 ($ _POST [ 'pass']); 

    // Check login. 
     if (@ mysql_query ("SELECT id FROM cp_users WHERE name = '". addslashes ($ user). "' AND pass = '". addslashes ($ pass). "' AND flag_enabled = '1 'LIMIT 1") & & @ mysql_affected_rows() == 1) 
     ( 
     if (isset ($ _POST [ 'remember']) & & $ _POST [ 'remember'] == 1) 
     ( 
      setcookie (COOKIE_USER, md5 ($ user), COOKIE_LIVETIME, CP_HTTP_ROOT); 
      setcookie (COOKIE_PASS, $ pass, COOKIE_LIVETIME, CP_HTTP_ROOT); 
     ) 

     LockSession(); 
     $ _SESSION [ 'Name'] = $ user; 
     $ _SESSION [ 'Pass'] = $ pass; 
     // UnlockSession(); 

     header ('Location:'. QUERY_STRING_BLANK. 'home'); 
    ) 
     else ShowLoginForm (true); 
     die(); 
    ) 

    ShowLoginForm (false); 
    die(); 
) 

// Output 
    if (strcmp ($ _GET [ 'm'], 'logout') === 0) 
    ( 
    UnlockSessionAndDestroyAllCokies(); 
    header ('Location:'. QUERY_STRING_BLANK. 'login'); 
    die(); 
) 
) 

////////////////////////////////////////////////// ///////////////////////////// 
// Check the login data. 
////////////////////////////////////////////////// ///////////////////////////// 

$ logined = 0,// flag means, we zalogininy. 

// Log in session. 
LockSession(); 
if (! empty ($ _SESSION [ 'name']) & &! empty ($ _SESSION [ 'pass'])) 
( 
    if (($ r = @ mysql_query ("SELECT * FROM cp_users WHERE name = '". addslashes ($ _SESSION [' name'])."' AND pass = ' ". addslashes ($ _SESSION [' pass']). " 'AND flag_enabled = '1' LIMIT 1 ")))$ logined = @ mysql_affected_rows(); 
) 
// Login through cookies. 
if ($ logined! == 1 & &! empty ($ _COOKIE [COOKIE_USER]) & &! empty ($ _COOKIE [COOKIE_PASS])) 
( 
    if (($ r = @ mysql_query ("SELECT * FROM cp_users WHERE MD5 (name)='". addslashes ($ _COOKIE [COOKIE_USER ])."' AND pass = '". addslashes ($ _COOKIE [COOKIE_PASS]). " 'AND flag_enabled = '1' LIMIT 1 ")))$ logined = @ mysql_affected_rows(); 
) 
// Unable to login. 
if ($ logined! == 1) 
( 
    UnlockSessionAndDestroyAllCokies(); 
    header ('Location:'. QUERY_STRING_BLANK. 'login'); 
    die(); 
) 

// Get the user data. 
$ _USER_DATA = @ Mysql_fetch_assoc ($ r); 
if ($ _USER_DATA === false) die (mysql_error_ex()); 
$ _SESSION [ 'Name'] = $ _USER_DATA [ 'name']; 
$ _SESSION [ 'Pass'] = $ _USER_DATA [ 'pass']; 

// Connecting language. 
if (@ strlen ($ _USER_DATA [ 'language'])! = 2 | |! SafePath ($ _USER_DATA [ 'language']) | |! file_exists ('system/lng .'.$_ USER_DATA [' language '].' . php'))$_ USER_DATA [ 'language'] = DEFAULT_LANGUAGE; 
require_once ('system/lng .'.$_ USER_DATA [' language'].'. php '); 

UnlockSession(); 

Answer 1

Sí, hay algunas vulnerabilidades en el código.

Esto podría ser un problema:

define ('QUERY_SCRIPT', basename ($ _SERVER [ 'PHP_SELF'])); 

PHP_SELF es malo porque un atacante puede controlar esta variable. Por ejemplo, intente imprimir PHP_SELF cuando acceda al script con esta url: http://localhost/index.php/test/junk/hacked. Evite esta variable tanto como sea posible, si la usa, asegúrese de desinfectarla. Es muy común ver que XSS aparece al usar esta variable.

primera vulnerabilidad:

setcookie (COOKIE_USER, md5 ($ user), COOKIE_LIVETIME, CP_HTTP_ROOT); 
setcookie (COOKIE_PASS, $ pass, COOKIE_LIVETIME, CP_HTTP_ROOT); 

Ésta es una vulnerabilidad más grave. Si un atacante tiene inyección SQL en su aplicación, entonces podrían obtener el hash md5 y el nombre de usuario e iniciar sesión inmediatamente sin tener que romper el hash md5(). Es como si estuviera almacenando contraseñas en texto claro.

Esta vulnerabilidad de sesión es doble, también es una "sesión inmortal". Los ID de sesión siempre deben ser grandes valores generados aleatoriamente que caducan. Si no caducan, entonces son mucho más fáciles de usar con la fuerza bruta.

Usted debe NUNCA re-inventar la rueda, llame session_start() en el comienzo de su aplicación, lo que generará automáticamente un identificador de sesión seguro de que expire. A continuación, utilice una variable de sesión como $_SESSION['user'] para realizar un seguimiento si el navegador está realmente conectado

segunda vulnerabilidad:.

$ pass = md5 ($ _POST [ 'pass']); 

md5() ha demostrado ser inseguros porque las colisiones se han generado intencionalmente. md5() debería nunca ser utilizado para contraseñas. Deberías usar un miembro de la familia sha2, sha-256 o sha-512 son excelentes opciones.

tercera vulnerabilidad:

CSRF

no veo ninguna protección CSRF para que la autenticación de la lógica. Sospecho que todas las solicitudes en su aplicación son vulnerables a CSRF.

Answer 2

La respuesta aceptada falta mucho y está equivocada sobre algunas cosas. Aquí están las vulnerabilidades que veo en el código:

define('QUERY_SCRIPT', basename($_SERVER['PHP_SELF'])); 

Como dije en otro lugar, esto puede contener algo más que la ruta del script. Use $_SERVER['SCRIPT_NAME'] o __FILE__ en su lugar.

define('CP_HTTP_ROOT', ... 

Esta constante se utiliza para establecer la ruta de la cookie a la ruta del script. Esto no es inseguro, pero el usuario deberá iniciar sesión por separado en cada secuencia de comandos. En su lugar, use las sesiones (que se describen a continuación) y establezca una ruta base para su aplicación.

UnlockSessionAndDestroyAllCokies() 

No sé exactamente qué está haciendo esto, pero no suena bien. Simplemente inicie la sesión anticipadamente en el script para cada solicitud. Compruebe si existe información del usuario en la sesión de saber si ya está conectado.

$pass = md5($_POST['pass']); 

Las contraseñas deben tener una sal única con cada uno de hash y es preferible utilizar un algoritmo de hash mejor. En estos días (PHP 5.5+) debe usar password_hash y password_verify para ocuparse de los detalles de hashing de contraseña para usted.

mysql_query("SELECT id FROM cp_users WHERE name = '". addslashes($user)... 

Ésta es una vieja pregunta, pero hoy en día los mysql_ funciones de PHP ya no se admiten. Use mysqli o PDO. El uso de una biblioteca no compatible lo deja abierto a las vulnerabilidades no parcheadas. addslashes no es una protección perfecta contra la inyección SQL. Utilice las declaraciones preparadas, o al menos las funciones de escape de cadenas de la biblioteca.

if (isset($_POST['remember']) && $_POST['remember'] == 1) 
( 
    setcookie(COOKIE_USER, md5($user), COOKIE_LIVETIME, CP_HTTP_ROOT); 
    setcookie(COOKIE_PASS, $pass, COOKIE_LIVETIME, CP_HTTP_ROOT); 
) 

Y aquí está el mayor problema. Las cookies están almacenando credenciales de usuario. Está enviando un nombre de usuario hash y una contraseña hash como valores de cookie con la respuesta. Estos pueden ser leídos y utilizados fácilmente por un tercero. Dado que este script usa un hash simple, una tabla rainbow le permitirá a alguien buscar muchas de las contraseñas de los usuarios. Pero dado que la respuesta contiene las credenciales "seguras", un atacante no necesitaría hacer otra cosa que pasarlas a cualquier otra solicitud para iniciar sesión. Esta no es la manera correcta de "recordar" a un usuario y no es necesario.

La conexión de un usuario a una solicitud debe manejarse solo en la sesión. Ese es su propósito. Las solicitudes y respuestas contienen una cookie con un identificador aleatorio. Los detalles del usuario permanecen solo en el servidor y el enlace a este identificador. (Nota al margen:. Hay un error que este bloque se envuelve con paréntesis en lugar de los apoyos)

$_SESSION['Pass'] = $pass; 

Ahora la contraseña del usuario se está almacenando en dos lugares: la base de datos y una sesión de almacenamiento. Si las sesiones se almacenan fuera de la base de datos (y PHP las almacena en el disco de manera predeterminada), ahora hay dos maneras en que alguien puede intentar robar las credenciales del usuario.

if ($logined !== 1 && !empty($_COOKIE[COOKIE_USER]) ... 
    if (($r = @mysql_query("SELECT * FROM cp_users WHERE MD5(name)='". addslashes($_COOKIE [COOKIE_USER ])."' AND pass = '". addslashes($_COOKIE [COOKIE_PASS]).. 

Un atacante puede ahora intentar conectarse simplemente pasando cabeceras de galletas en una solicitud con un hash MD5 de un nombre de usuario y contraseña, que fue entregado a ellos en una respuesta anterior. El formulario de inicio de sesión debe ser el único lugar que tome las credenciales del usuario y las inicie sesión. Este formulario (y todos los demás) deben usar un token CSRF.

UnlockSession(); 

Una vez más no sé qué está haciendo, pero al final de la secuencia de comandos de la sesión solo se deben escribir en el almacenamiento.