Not sure if sql or php error

Question

I am currently following a tutorial on Youtube called Register & Login/PHP tutorials by Alex from Phpacademy.. am in part 5 and here is login.php

<?php
include 'core/init.php';

if (empty($_POST) === false) { 

    $username = $_POST['username']; 
    $password = $_POST['password']; 

    if (empty($username) === true || empty($password) ===   true) {

        $errors[]  = 'You need to enter a username and password ';

    } else if (user_exists($username) === false) {
        $errors[] = 'We couldn\'t find that username. Have you registered?';
    } 
    else if (user_active($username) === false){
        $errors[] = 'You havn\'t activated your account!';  
    } 
    else  { 
        $login = login($username, $password);
        if ($login === false) {
            $error[] = 'That username/password combination is incorrect';
        } else {
        $_SESSION['user_id'] = $login;
        header('Location: index.php');
        exit();
        }
    } 

}

print_r($errors);

?>

Here is users.php

<?php

function user_exists($username) { 
$username = sanitize($username); 
return (mysql_result(mysql_query("SELECT COUNT(`user_id`) FROM `users` WHERE `username` = '.$username'"), 0) == 1) ? true : false;
}

function user_active($username) { 
$username = sanitize($username); 
return (mysql_result(mysql_query("SELECT COUNT(`user_id`) FROM `users` WHERE `username` = '.$username'  AND `active` = 1 ") , 0 ) == 1 ) ? true : false;

}

function user_id_from_username($username){

    $username = sanitize($username);
    return mysql_result (mysql_query("SELECT `user_id` FROM `users` WHERE `username` = '$username' "), 0, 'user_id');

}
function login($username, $password){

    $user_id = user_id_from_username($username);
    $username = sanitize($username);
    $password = md5($password);
    return (mysql_result(mysql_query("SELECT COUNT(`user_id`) FROM `users` WHERE `username` = '.$username' AND `password` = '.$password'"), 0) == 1) ? $user_id : false;
}
?>

and here is the output Array ( [0] => We couldn't find that username. Have you registered? )

Am new here, apologies in advance

**warning** your code is vulnerable to sql injection attacks. — Daniel A. White, Sep 23 '13 at 19:14
also, don't just do an md5 to store your passwords. include a salt. — Daniel A. White, Sep 23 '13 at 19:15
@DanielAWhite how do you know? he hasn't shared his sanitize method — ddavison, Sep 23 '13 at 19:16
The above are important and valid, but since he's following a tutorial series and it's not far in, getting basic functionality working is the goal, security comes later in the series — Chris Brown, Sep 23 '13 at 19:16
Find a newer tutorial. mysql_* functions are deprecated and about to be removed from the language. — TecBrat, Sep 23 '13 at 19:16
Related and useful for you: http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php — PeeHaa, Sep 23 '13 at 20:02

score 0 · Answer 1 · answered Sep 23 '13 at 19:17

0

WHERE `username` = '.$username' AND `password` = '.$password'"

Remove the dots

answered Sep 23 '13 at 19:17

Mihai

26,325
7
66
81

I did that on purpose because it showed me this Warning: mysql_result() expects parameter 1 to be resource, boolean given in C:\xampp\htdocs\Core\users.php on line 10 Array ( [0] => You havn't activated your account! ) I have already changed the value to 1 – ATP Sep 23 '13 at 19:21

score 0 · Answer 2 · answered Sep 23 '13 at 19:18

Your SQL queries are going to be returning bad results. Otherwise, you will be searching for .jond in your database if the username they entered is jond.

return (mysql_result(mysql_query("SELECT COUNT(`user_id`) FROM `users` WHERE `username` = '.$username'"), 0) == 1) ? true : false;

Remove the . before $username and $password in the query.

"SELECT COUNT(`user_id`) FROM `users` WHERE `username` = '$username'"

score 0 · Answer 3 · answered Sep 23 '13 at 19:18

Your query needs a tad bit tweaking. Remove the period in front of the username since it's inside the double quotes

return (mysql_result(mysql_query("SELECT COUNT(`user_id`) FROM `users` WHERE `username` = '$username'"), 0) == 1) ? true : false;

This goes the same for the other queries in that file. As mentioned in the comments, you really ought to switch from the deprecated mysql_* functions to PDO/mysqli so that your code will still work in future versions of PHP, and you won't be open to injection hacks.

score 0 · Answer 4 · answered Sep 23 '13 at 19:19

Your code is pretty hideous overall. You should NOT be nesting your mysql calls like that. Nesting like that implies that you think a DB operation will NEVER fail. This is a VERY BAD assumption.

That being said, here's at least one source of your problems:

return (...snip ... WHERE `username` = '.$username'"), 0) == 1) ? true : false;
                                        ^--- here

You've embedded a . in that query, making all your usernames look like .foo instead of just foo. The problem exists in both user_exists(), user_active() AND login().

I did that on purpose because it showed me this Warning: mysql_result() expects parameter 1 to be resource, boolean given in C:\xampp\htdocs\Core\users.php on line 10 Array ( [0] => You havn't activated your account! ) I have already changed the value to 1 — ATP, Sep 23 '13 at 19:26

Not sure if sql or php error

4 Answers4