Are there any security vulnerabilities in this PHP code?

Question

I just got a site to manage, but am not too sure about the code the previous guy wrote. I'm pasting the login procedure below, could you have a look and tell me if there are any security vulnerabilities? At first glance, it seems like one could get in through SQL injection or manipulating cookies and the ?m= parameter.

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

Yes, there are a few vulnerabilities in this code.

This could potentially be a problem:

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

PHP_SELF is bad because an attacker can control this variable. For instance try printing PHP_SELF when you access the script with this url: http://localhost/index.php/test/junk/hacked . Avoid this variable as much as possible, if you do use it, make sure you sanitize it. It is very common to see XSS crop up when using this variable.

1st Vulnerability:

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

This is a rather serious vulnerability. If an attacker had SQL injection in your application then they could obtain the md5 hash and the user name and login immediately without having to break the md5() hash. It is as if you are storing passwords in clear text.

This session vulnerability is two fold, it is also an "immortal session", Session id's must always be large randomly generated values that expire. If they don't expire then they are much easier to brute force.

You should NEVER re-invent the wheel, call session_start() at the very start of your application and this will automatically generate a secure session id that expires. Then use a session variable like $_SESSION['user'] to keep track if the browser is actually logged in.

2nd vulnerability:

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

md5() is proven to be insecure because collisions have been intentionally generated. md5() should never be used for passwords. You should use a member of the sha2 family, sha-256 or sha-512 are great choices.

3rd Vulnerability:

CSRF

I don't see any CSRF protection for you authentication logic. I suspect that all requests in your application are vulnerable to CSRF.