Friends tell me if I wrote the code correctly, authorization. Ajax data processing handler:

if ($_GET['action'] == 'getLogin') { $user = addslashes(htmlspecialchars($_POST["user"], ENT_QUOTES, '')); $pass = $_POST['pass']; $get_pass = mysql_query("SELECT id, login, pass, status, active FROM users WHERE login = '".$user."' LIMIT 1"); $row = mysql_fetch_array($get_pass); $id = $row['id']; $login = $row['login']; $user_password = $row['pass']; $status = $row['status']; $mail_conf = $row['active']; if(as_md5($key, $pass) != $user_password || !$login) { $login = ''; print 'Данные введены неверно'; } elseif($mail_conf == 1) { print 'Ваш E-mail не подтвержден'; } elseif($status == 3) { print 'Ваш счет временно заблокирован. Обратитесь в службу поддержки '; } elseif($status == 4) { print 'Вам запрещено больше принимать участие в проекте'; exit(); } else { session_start(); $_SESSION['login'] = $user; $ip = getip(); $time = time(); mysql_query("UPDATE users SET ip = '".$ip."', go_time = ".$time." WHERE login = '".$login."' LIMIT 1"); mysql_query("INSERT INTO logip (user_id, ip, date) VALUES (".$id.", '".$ip."', ".$time.")"); print "Переходим в кабинет..."; print "<script language=\"javascript\">setTimeout(function(){top.location.href=\"/myprofile/\";}, 1500); </script>"; } }

Ajax processing:

 $(document).ready(function(){ $('#LoginForm').submit(function(){ $('#loading-formLogin').css('visibility','visible'); var data = { user: $('#user').val(), pass: $('#pass').val() } setTimeout(function(){ $.ajax({ type: "POST", url: "/log-form.php?action=getLogin", data: data, success: function(html){ $('#loading-formLogin').css('visibility','hidden'); $('#LoginResult').fadeIn(200).css('display','block'); setTimeout(function(){ $('#LoginResult').fadeOut(1000).css('display','block'); }, 1500); $('#LoginResult').html(html).css('display','block'); } }); }, 1000); return false; }); });

Everything is working fine for me. If something is wrong in the code, please indicate errors) I would be very grateful!

Closed due to the fact that it is necessary to reformulate the question so that it was possible to give an objectively correct answer to the participants Andrei Arshinov , Athari , petya , Maxim Kamalov , Duck Learns to Hide 10 May '15 at 15:05 .

The question gives rise to endless debates and discussions based not on knowledge, but on opinions. To get an answer, rephrase your question so that it can be given an unambiguously correct answer, or delete the question altogether. If the question can be reformulated according to the rules set out in the certificate , edit it .

  • It is necessary to use the PDO library to work with the database, it is more reliable and safer - modelfak
  • Try to write more detailed questions. Explain exactly what you see the problem, how to reproduce it, etc. - Nicolas Chabanovsky ♦

2 answers 2

In general, nothing special, except that the code is completely barbaric. So they wrote 20 years ago, in the last century. For example, the whole horror of the first 10 lines is reduced in our time to one:

 $user = User::find_by_login($_POST["user"]);

when working with a database through ORM.

Plus Md5 is considered a bad choice for password hashing. Password Hashing should be used.

Also anachronism is sending text to the browser. JSON is commonly used now. For example, sending a javascript with all these sticks and quotes looks awful at all. Why it is impossible to register the same code in the template, and if a message about success comes from PHP, success just call it?

Well, I am 99% sure of the sql injection through $ip , because, again, unacceptable tools for working with the database and the barbaric function getip() - have not done others in the last century.

If you do not want to use third-party libraries to work with the database, then there is only one option to work with pure SQL - PDO . It should be understood that by the fact of its presence PDO does not protect any data. Protection is guaranteed only when 100% of the data, with no exceptions , are inserted into the request through placeholders. For example, the last two queries will look like this:

 $sql = "UPDATE users SET ip = ? go_time = ? WHERE login = ?"; $pdo->prepare($sql)->execute(array($ip, $time, $login)); $pdo->prepare("INSERT INTO logip VALUES (?,?,?)")->execute(array($id, $ip, $time));

Instead of the getip() function, $_SERVER['REMOTE_ADDR'] should be used $_SERVER['REMOTE_ADDR']

    I can not comment, but I disagree with Shorty a bit. I and many others use Mysqli instead of PDO. And Mysqli is the recommended choice. The only problem I encountered was the IN in the query with regular expressions - WHERE query with IN in Mysqli - how? But this problem is solvable, and this problem is also present in the PDO. Since the optimal solution to the problem with IN queries is SafeMySQL , I would recommend this particular library to you. On forums I often meet recommendations to use SafeMySQL .

    Regarding md5, I fully support Shorty. Need to use Password Hashing. If you have php> = 5.3.7, but less than 5.5, you can use this library for Password Hashing: https://github.com/ircmaxell/password_compat

    • Friends, but I will not have a social network where several million people will go) I just write simple scripts for myself, so I learn)) Thanks to everyone! - Vyacheslav
    • Well, you yourself asked for advice. And it’s better to learn right away :) - Denis