PHP authentication script not working, why?

[johnwhello]

This is my authentication script. Why isn't it working:


// member_authentication.php doesn't work, why?

<?php
function checking($name, $passwordPost, $admin) {
    if ($admin == false) {
            $sql = "SELECT klant_id, username, password FROM klant WHERE username='$name'";
    } else {
        $sql = "SELECT klant_id, username, password FROM klant LIMIT 0,1";
    }
    $result = mysql_query($sql);
    if (mysql_num_rows($result) > 0) {
        $passwordDb = mysql_result($result, 0, "password");
        $clientid = mysql_result($result, 0, "klant_id");
        if ($passwordPost != $passwordDb) {
            return false;
        } else {
            return $clientid;
        }
    } else {
        return false;
    }
}
// Check name and password
if (isset($_POST['sendButton2'])) {
    $usernamePost = $_POST['usernamePost'];
    $passwordPost = md5($_POST['passwordPost']);
    $clientid = checking($usernamePost, $passwordPost, $admin);
    if ($clientid != false) {
        $_SESSION['usernamePost'] = $usernamePost;
        $_SESSION['passwordPost'] = $passwordPost;
        $_SESSION['clientid'] = $clientid;
    }
}
if (checking($_SESSION['usernamePost'], $_SESSION['passwordPost'], $admin) == false) {
    echo "<form method=\"POST\" action=\"".$_SERVER["PHP_SELF"]."?";
    reset($_GET);
    // send already sent variables again
    while($getVar = each($_GET)) {
        $varName = $getVar['key'];
        $varContent = $getVar['value'];
        echo "$varName=$varContent&";
    }
    echo "\">\n";
    echo "Name: ";
    echo "<input type=\"text\" name=\"usernamePost\">";
    echo "<br>";
    echo "Password: ";
    echo "<input type=\"password\" name=\"passwordPost\">";
    echo "<br>";
    echo "<input type=\"submit\" value=\"Send\" name=\"sendButton2\">";
    echo "</form>";
    echo "<p />Not yet registered? <a href=\"page_register3.php\">[Register here]</a><br />";
    if ($admin == "true") {
        echo "<p />--- Admin level is required!";
    }
    exit;
}
?>

The output in is shown here:
http://www.michaellobry.com/member_cart.php
- after entering username+password, nothing happens, except for a question mark then gets added after the URL:
http://www.michaellobry.com/member_cart.php?

It should become:
http://www.michaellobry.com/member_c...lah&value=blah

[3PRIMATES]

Hi, well for certian something is going on. View the source of your login page.
Your html stops right after the form.

Are you forgetting to include a footer page or something?

3P

[jasonsplace]

If you want the variables appended to the end of the url then you need to change the form method from POST to GET.

[johnwhello]

The script is correct, also it has to be <form method="POST"

[johnwhello]

The script is correct, also it has to be <form method="POST">

I think it's a naming issue (double or wrong linked name) (of the Database fields, variable names and input field names). But after 12 hours trying, i still can't find out which name should be changed.

[jasonsplace]

Ok. I see that you are adding the GET variables with your loop. When you use reset($_GET) it clears out all of the GET variables right before looping through so there is nothing for it to add. Try getting rid of that line or moving it to after the loop.

[Musicman]

Hi,

for proper use of each(), reset() and list() check the php manual http://de.php.net/manual/de/function.each.php - reset does not clear the data but ensures the loop runs from the first item
There is a different way to do that, too:

foreach($_GET as $key => $value)
     echo "$key=" . urlencode($value) . "&";

Of course this stuff will only work if GET variables are used - what kind of products should I try to buy to see some?

Musicman

[johnwhello]

That's a wrong statement about reset. It's true that 'reset' sets the pointer to zero, but:
reset($_GET);
and all the code are set in the right position!, according to my PHP book page 214. The only thing changed are the names, that's why i mentioned that it's a naming error issue.

Also, after this 'reset', the values and keys are added again (from zero to the amount of keys there are (coming from the <form> fields)).

[jasonsplace]

Sorry. I misread reset as unset for some reason. I need to start going to bed at night.

[johnwhello]

Also see this URL (of this code):
http://www.michaellobry.com/member_cart.php
it outputs 2 errors as you see (as follows:

Notice: Undefined index: usernamePost in /home/vhosts/michaellobry.com/httpdocs/member_authentication.php on line 34

Notice: Undefined index: passwordPost in /home/vhosts/michaellobry.com/httpdocs/member_authentication.php on line 34

It sais that line 34 contains the error. Line 34 is this:
if (checking($_SESSION['usernamePost'], $_SESSION['passwordPost'], $admin) == false) {

But all code are correct and variables are correctly linked.

[Musicman]

Hi,

lets say the code is semi-correct - it reads out the saved session data and uses that to check.
Now, on most servers, undefined data would silently be replaced with blank, and the blank user would fail to verify against the database.
Once you successfully login, the messages should go away.

Now, you could
- change error reporting
- add a few @ signs to your code where you expect errors
- use an explicit test, like

if(!isset($_SESSION['usernamePost']) || !checking($_SESSION['usernamePost'], ...)

The GET parameters will only show up if there are any, e.g. if you click on a product and the link would do something like
member_cart.php?add=4711

Musicman

[johnwhello]

Thanks you all!
Now the script works (i got it on my own).

The error lay in this code:

if ($passwordPost != $passwordDb) { 

That code should have been (and then the code works as you see at: http://www.michaellobry.com/member_admin.php (type in, username: c, password: d)

if ($_POST["passwordPost"] != $passwordDb) { 

because, the typed in password, must be checked whether it equals the password in the database (=$passwordDb)

[johnwhello]

Hi,

lets say the code is semi-correct - it reads out the saved session data and uses that to check.
Now, on most servers, undefined data would silently be replaced with blank, and the blank user would fail to verify against the database.
Once you successfully login, the messages should go away.

Now, you could
- change error reporting
- add a few @ signs to your code where you expect errors
- use an explicit test, like

if(!isset($_SESSION['usernamePost']) || !checking($_SESSION['usernamePost'], ...)

The GET parameters will only show up if there are any, e.g. if you click on a product and the link would do something like
member_cart.php?add=4711

Musicman

Thanks, you are right. Only after clicking on a product, the GET parameter proceeds.

[Hysteresis]

This code has some major problems. Firstly, you have a gaping wide open SQL injection hole. The $_POST['postUserName'] value is passed unfiltered in the database query. The first rule of backend design is to never ever trust any user input. You have to run all user input through mysql_real_escape_string() before using it in a query or you're just asking to get owned.

Secondly, try putting something like "?%22onSubmit=%22alert('Simple PoC')%22" at the end of your URL and load the page. When you submit the form you should get a popup saying 'Simple PoC'. Again this is a problem with you trusting user input and not filtering. Any user input that gets put on the page must be passed through htmlentities() or it will lead to XSS holes, like the one I've just shown.

I suggest you invest some time learning about security practices if you're going to develop mission critical code - especially where customer's money and information is involved as with a shopping cart. Code security must be the number one priority in these situations, as you can have all the bells and whistles you like, but if people don't trust their money or information with you it isn't going to matter what cool features you have.

[Musicman]

Hi Hysteresis,

where johnwhello lives, the php magic quotes provide good protection against sql attacks. In some language settings, only mysql_real_escape_string is safe

Musicman