How to make this login unvulnerable

Question

They have told me this log in is vulnerable, but i dont know how to make it safe, im looking for help if anyone whants to help me I'd really appreciate it.

When i add this: "1' OR 1=1 LIMIT 1#" to the password en the login form it enters.

Here is the code:

<?php
session_start();
include 'inc/header.php';
include 'panel_funciones.php';

$usuario = $_POST["nombre"];
$pass = $_POST["pass"];
try {
 $bd = new PDO("mysql:host=localhost;dbname=b9_16267033_1","b9_16267033","12346");
 $bd->query("SET NAMES 'utf8'");
} catch (Exception $e){
 echo "No se ha podido conectar";
 exit;
}
try{
        $sql= "SELECT usuario, pass FROM usuarios WHERE usuario='$usuario' and pass='$pass'";
}catch(Exception $e){
 echo "Error en consulta";
 exit;
}
$iniciosesion = $bd->query($sql);
$result = $iniciosesion->fetchAll();
$contar = count($result);

// AQUI COMIENZA COMPROBACIÓN
if ($_SESSION['Logueado'] = TRUE) {
 panel();
}
elseif ($contar == 1) {
 $_SESSION['Logueado'] = TRUE;
panel();
}
else{
 echo "El usuario o contraseña es incorrecto";
}
include 'inc/footer.php';
?>

*Unvulnerable* is probably a missformulation. It is hard to *prove* one cannot hack a login application.. — Willem Van Onsem, May 30 '15 at 22:47
@JeroenSmit: furthermore the passwords aren't hashed either... — Willem Van Onsem, May 30 '15 at 22:55

score 1 · Accepted Answer · edited May 30 '15 at 23:23

1

There are several well known problems with this page:

SQL injection
```
$sql= "SELECT usuario, pass FROM usuarios WHERE usuario='$usuario' and pass='$pass'";
```
The problem is that you use text processing to insert something in the query. Now say I modify my password and specify it as ' or 'a'='a', in that case I get entrance. Because 'a'='a'. Furthermore it allows me to do all kinds of queries. Like '; DROP TABLE usuarios. You better always use prepared statements (or something equivalent) where you don't enter the parameters yourself. The prepared statements will escape the parameters such that one cannot inject SQL into these.

You should use prepared statements; something like:
```
$sql= "SELECT usuario, pass FROM usuarios WHERE usuario=? and pass=?";
$stm = $bd->prepare($sql);
$stm->execute(array($usuario,$pass));
$result = $stm->fetchAll();
$contar = count($result);
```
Unhashed passwords: here is a manual on password hashing. Never store passwords itself. Say a hacker has found a weak spot in your website and somehow has managed access to your database, then you are lost. The hacker can copy all the passwords and easily modify them. Furthermore there are a lot of users that use the same password on all applications making it even easier to hack into other websites.

A third aspect you found yourself is the assignment in the if statement:

if ($_SESSION['Logueado'] = TRUE) {
    panel();
} elseif ($contar == 1) {
    $_SESSION['Logueado'] = TRUE;
panel();
} else{
    echo "El usuario o contraseña es incorrecto";
}

Here you assign TRUE to your $_SESSION variable. A better way is the following:

if($contar == 1) {
    $_SESSION['Logueado'] = TRUE;
}
if($_SESSION['Logueado']) {
    panel();
} else{
    echo "El usuario o contraseña es incorrecto";
}

Another aspect: you cannot make a page unvulnerable: chances are great that hackers will find ways to circumvent a lot of protective measures eventually. Security is not a yes-or-no. The point is that you must make it that much hard that most hackers will fail in bypassing security.

edited May 30 '15 at 23:23

Javier Benito Santoni

57
7

answered May 30 '15 at 22:55

Willem Van Onsem

443,496
30
428
555

If i add this: "1' OR 1=1 LIMIT 1#" to password it enters to the system – Javier Benito Santoni May 30 '15 at 23:13
Using prepared statements? – Willem Van Onsem May 30 '15 at 23:13
I think this happens because of $_SESSION['Logueado'] = TRUE im very new at this, where can i find the one that works for this. – Javier Benito Santoni May 30 '15 at 23:14
@JaviB: yes, that's another problem, you should use `==` – Willem Van Onsem May 30 '15 at 23:15
If i use == it dosnt work, its always saying El usuario o contraseña es incorrecto (The user or password its not correct) when i type the correct one. – Javier Benito Santoni May 30 '15 at 23:18
@JaviB: updated with a *bugfix*. Does it work now? Mind that you will have to remove `$_SESSION` first. – Willem Van Onsem May 30 '15 at 23:18
Edit post, you should put here elseif instead of: if($_SESSION['Logueado']) Thanks for your help :) – Javier Benito Santoni May 30 '15 at 23:27
@JaviB: but in that case at the login process, the `panel()` will not be called. – Willem Van Onsem May 30 '15 at 23:29
If that is not done it cant check if your session is open or not – Javier Benito Santoni May 30 '15 at 23:30
Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/79227/discussion-between-javi-b-and-commusoft). – Javier Benito Santoni May 30 '15 at 23:30

How to make this login unvulnerable

1 Answers1