Simple if() not working as expected

Question

I am sure that I am simply overlooking something, and I spent a few days working on this and cant seem to figure it out.

after logging in on the previous page I get the username and password,

$username = mysql_escape_string($_POST['adminusername']);
$password = mysql_escape_string($_POST['adminpassword']);

and then I go to the database to pull the username and password from the database,

$sql = "SELECT username, password FROM `weaponsadmin`";
$rows = $db->query($sql); while ($record = $db->fetch_array($rows)) {

now here is the part that is confusing me, if i have the following, no matter what I use for the username or password, it will NOT allow for me to login,

if ( ($record[username]==$username) && ($record[password]==$adminpassword) ){
   $_SESSION['loggedin'] = true;
   $_SESSION['username'] = $adminusername;
   header( "Location: admin.php" ) ;
}
else {
   header( "Location: index.php?login=error" ) ;
}

however if I use the following, it will allow me to login in if the username is correct, but it allows for me to input anything for password and it works,

$adminusername = $record[username]; 
$adminpassword = $record[password];

if ( ($adminusername==$username) && ($adminpassword==$adminpassword) ) {
   $_SESSION['loggedin'] = true;
   $_SESSION['username'] = $adminusername;
   header( "Location: admin.php" ) ;
}
else {
   header( "Location: index.php?login=error" ) ;
}

So in summary for some reason the && part doesn't seem to work correctly and if somebody could help me with the code and let me know where my code could be improved for better security and how to make this work correctly, thanks

Array indexes should be integers or strings, so `$record[username]` should be `$record['username']`. Without the quotes, PHP is trying to find the `username`/`password` constants. Docs: http://php.net/manual/en/language.types.array.php — Jasper, Jun 15 '12 at 19:41
@Wrikken yup that is the problem, thanks i knew it was something simple — Chris James Champeau, Jun 15 '12 at 19:56

score 3 · Accepted Answer · answered Jun 15 '12 at 19:42

3

what is the point here $adminpassword==$adminpassword :

i think it should be:

if ( ($adminusername==$username) && ($adminpassword==$password) ){

answered Jun 15 '12 at 19:42

mgraph

15,238
4
41
75

This would also explain why the first example never worked, because in that scenario `$adminpassword` had never been set – Chris James Champeau Jun 15 '12 at 19:57

score 1 · Answer 2 · answered Jun 15 '12 at 19:41

1

$record[username] should be $record["username"] (and so on). indexes are strings or int

answered Jun 15 '12 at 19:41

Alfabravo

7,493
6
46
82

score 1 · Answer 3 · answered Jun 15 '12 at 19:57

You're using arrays wrong.
You expect: $record[username]; //retrieve contains of key "username"
What really happens:

    $record[username]; 
    /* 
         retrieves a key in the record array under the key which is a value of a
         constant named "username" (if it's defined) and an empty string with 
         E_WARNING if it's not. 
    */

You need to either single or double quote the index names, for example $records['username']. However, you can use unquoted array indexes inside of a string (and these will work as you expect) -> $someString = "Blahblahblah, ergo $record[username] is a donkey.";.

score 0 · Answer 4 · edited May 23 '17 at 12:15

0

You can use === instead of ==. Read this.
strcmp() isn't necesarry here.

edited May 23 '17 at 12:15

Community

1
1

answered Jun 15 '12 at 19:54

Will using identity operator instead of equality really solve his problem? – cypher Jun 15 '12 at 19:59
This code was the problem: `$adminpassword==$adminpassword`, like mgraph said before, and this: `$adminusername = $record[username]; $adminpassword = $record[password];`, but is safer to use `===` – Jun 16 '12 at 11:09

score 0 · Answer 5 · answered Jun 15 '12 at 19:55

To add to Michael's answer, the reason why you should not use == for string comparison (hopefully this will help you navigate similar difficulties in the future) is that when you call a simple == on an object (such as a string, or really anything other than an int, double, float, char, long, short, or boolean, in most languages), what you're really comparing is the address in memory of each object, that is, the pointer value.

This is useful if you want to know if two variables are referencing the same object, but not so useful if you want to know if two objects are identical. So this is true:

$string_a = $some_string;
$string_b = $some_string;
$string_a == $string_b;

but this is not:

$string_a = getUserInput();  # user types in "hello"
$string_b = getUserInput();  # user types in "hello"
$string_a == $string_b;

and this may be true depending on the language you're in, if it stores string literals in memory independently of the user-defined variables to which they are attached:

$string_a = "hello";
$string_b = "hello";
$string_a = $string_b;

So unless you're checking to see if two objects are in fact the same object, and not merely identical, use, as those before me suggested, a function to compare the two. Such a function usually goes down to the level of primitive types, which can be compared using == as you would expect, and returns true if all of those comparisons do.

Why exactly should we not use `==` for string comparsion in php? — cypher, Jun 15 '12 at 19:57

score -1 · Answer 6 · edited Jun 16 '12 at 15:09

-1

Don't use == for string comparison. Instead, use strcmp() or === to match strings.

edited Jun 16 '12 at 15:09

octopusgrabbus

10,555
15
68
131

answered Jun 15 '12 at 19:45

Tschallacka

27,901
14
88
133

1

Why should we not use `==` for string comparsion? – cypher Jun 15 '12 at 20:00
Its inaccurate, easy to hack. just look in the php online docs at the operators section where the user comments are. there is explained in details why its unsafe. also googling php string compare will yield many warnings not to rely on == or === for many reasons. use strcmp, preg_match or any other reliable function to compare strings more reliable, especially when it concerns an admin password and username. – Tschallacka Jun 15 '12 at 21:14
1

== and === is perfectly reliable. None of this is true and using preg_match for string comparsion is really an riddiculous idea. – cypher Jun 16 '12 at 00:48

Simple if() not working as expected

6 Answers6