Basic Error?

[JonsAdventure]

Okay, so my website admin finally got back to me and I can now use my database. I just attempted to make a functioning Login, but with no success.
The issue seems to be the fact that the action tag in form (<form action="process.php">) opens the php script in another tab.
http://www.thefunnyzone.co.uk
Username: Jon Password: password.

[traq]

All I see is a blue page. If you need help with your PHP code, there's no point in showing us the page anyway: you need to share the code.

[Ashley Sheridan]

It sounds like you might have a target attribute set in the form tag. If this exists and is anything other than _self or _top then it may well be opening in a new tab.

[JonsAdventure]

At the start of the main php document there's an include to a file with this inside:

<form id="Process" name="Process" method="POST" action="">
    <fieldset>
        <label for="name" id="name_label">Username:</label>
        <input type="text" name="name" id="name" size="30" value="" class="text-input" />
        <label class="error" for="name" id="name_error">This field is required.</label><br/>
        <label for="password" id="password_label">Password:</label>
        <input type="password" name="password" id="password" size="30" value="" class="text-input" />
        <label class="error" for="password" id="password_error">This field is required.</label>
        <label class="error" id="login_error">Incorrect password or username.</label>
        <input type="submit" name="submit" class="button" id="submit_btn" value="Login" style="position:absolute; left:650px; top:0px;"/>
    </fieldset>
</form>

<?
    mysql_connect("[Removed]", "[Removed]", "[Removed]");  
    mysql_select_db("[Removed]")
    $password = $_POST['password'];
    $username = md5($_POST['username']);
    $result=mysql_query("SELECT * FROM Users WHERE Username='$username' and Password='$password'");
    $count=mysql_num_rows($result);
    if($count==1){
        $expires = 1 * 1000 * 60 * 60 * 24 * 2;
        setcookie("username", $username, time()+$expires);
        setcookie("password", $password, time()+$expires);
        echo '<strong>Welcome, <? print $my_username; ?>!'; 
    }
?>

[bradgrafelman]

$password = $_POST['password'];
    $username = md5($_POST['username']); 

So you're storing the password in plain text, but you're computing the MD5 hash of the username?

EDIT: I think the above oddity is the thing that first jumps out at me, but just to cover all the bases, here are some more issues I see:

1. Stop Using the MySQL Extension!
2. Your HTML form contains no input element named 'username'.
3. User-supplied data should never be placed directly into a SQL query string, else your code will be vulnerable to SQL injection attacks and/or just plain SQL errors. Instead, you must first sanitize the data (such as by using mysqli_real_escape_string() for string data) or by using prepared statements.
4. Your PHP code is always attempting to execute the SQL query and check for results - even if it's the first time the user has loaded the form and hasn't even submitted any login credentials yet.
5. You never check to see if the SQL query failed to execute before attempting to use the result set.
6. Don't use 'SELECT *' - instead, only SELECT the information that you actually need. Since in your case you don't really need information from the database (you just want to check if there is a row that matches the WHERE criteria), you could instead SELECT some constant value (e.g. do a 'SELECT 1').

7. echo '<strong>Welcome, <? print $my_username; ?>!';

   PHP code won't get executed if it's placed inside a string, so unless you meant to display to your user the word "Welcome" followed by a comma, a space, a less-than symbol, a question mark, ... etc., then you should either use concatenation or variable interpolation here instead. See the manual page for string for more explanation/examples.
8. Your current code doesn't do anything if the username and password are incorrect. Wouldn't it at least be helpful to the user if you output some error message if no matching row can be found in your DB? (Note you'll want to make sure you address issue #4 above first, otherwise you'll be saying the user's username/password combination is invalid before they've even given you one.)

[JonsAdventure]

Okay, 7 and 8 aren't a problem ; php runs like that in my documents, the errors are dealt by JavaScript/jQuery:




$(function() {
$('.error').hide();
$('input.text-input').css({backgroundColor:"#ECECEC"});
$('input.text-input').focus(function(){
$(this).css({backgroundColor:"#FFDDAA"});
});
$('input.text-input').blur(function(){
$(this).css({backgroundColor:"#ECECEC"});
});
$(".button").click(function() {
$('.error').hide();
var name = $("input#name").val();
if (name == "") {
$("label#name_error").show();
$("input#name").focus();
return false;
}
var password = $("input#password").val();
if (password == "") {
$("label#password_error").show();
$("input#password").focus();
return false;
}
});
});

[Ashley Sheridan]

Don't ever rely on Javascript validation, ever, ever, ever! Javascript can be turned off or be unavailable, and all users are evil bastards who want to destroy your server. Any kind of validation always needs to be done server side too, as that's the only area where you have full control.

[JonsAdventure]

Okay, I've made some edits, am I going on the right lines here?

<form id="Process" name="Process" method="POST" action="">
    <fieldset>
        <label for="name" id="name_label">Username:</label>
        <input type="text" name="name" id="name" size="30" value="" class="text-input" />
        <label class="error" for="name" id="name_error">This field is required.</label><br/>
        <label for="password" id="password_label">Password:</label>
        <input type="password" name="password" id="password" size="30" value="" class="text-input" />
        <label class="error" for="password" id="password_error">This field is required.</label>
        <label class="error" id="login_error">Incorrect password or username.</label>
        <input type="submit" name="submit" class="button" id="submit_btn" value="Login" style="position:absolute; left:650px; top:0px;"/>
    </fieldset>
</form>

<?
    $mysqli = new mysqli("", "", "", "");
    if ($mysqli->connect_errno > 0) {
        die('Unable to connect to database [' . $mysqli->connect_error . ']');
    }
    $username = $mysqli->real_escape_string($_POST['user']);
    $password = md5($mysqli->real_escape_string($_POST['password']));
    $mysql->query("SELECT `id` FROM `Users` WHERE `Username` = '$username' && `Password` = '$password'");
    $sql = <<<SQL SELECT * FROM `Users` WHERE `Username` = $username && `Password` = $password SQL;
    if(!$result = $db->query($sql)){
        die('There was an error running the query [' . $db->error . ']');
    }else($result->num_rows==1){
        $expires = 1 * 1000 * 60 * 60 * 24 * 2;
        setcookie("username", $username, time()+$expires);
        setcookie("password", $password, time()+$expires);
        echo '<strong>Welcome,'.$username.'!</strong>'; 
    };
    $mysqli->close();
?>

[traq]

<? 
    $mysqli = new mysqli("", "", "", ""); 
    if ($mysqli->connect_errno > 0) { 
        # what version of PHP are you running?
        # mysqli::connect_error() was broken before 5.2.9
        die('Unable to connect to database [' . $mysqli->connect_error . ']'); 
    } 
    $username = $mysqli->real_escape_string($_POST['user']); 
    # don't hash the escaped password.
    # actually, since md5 hashes never have characters that need to be escaped,
    # you can skip it if you like.
    # others here might point out that md5 is considered "broken" for security purposes nowadays.
    $password = md5($mysqli->real_escape_string($_POST['password'])); 
    # why two queries?  why not just one, and see if it's empty or not?
    $mysql->query("SELECT `id` FROM `Users` WHERE `Username` = '$username' && `Password` = '$password'"); 
    # this WON'T WORK.
//    $sql = <<<SQL SELECT * FROM `Users` WHERE `Username` = $username && `Password` = $password SQL; 
    # a heredoc needs to be like this:
    $sql = <<< SQL
SELECT * FROM `Users` WHERE `Username` = '$username' && `Password` = '$password'
SQL
;
    # explanation:
    # opening token needs to be on its own line. 
    # likewise with the closing token, 
    #   which also must NOT be indented or have any other characters (even whitespace) on the same line
    # (technically, the closing ; can *sometimes* be included, 
    #   but for simplicity I just always put it on the following line).
    # also note I added 'quotes' to the string values in your SQL.
    if(!$result = $db->query($sql)){ 
        die('There was an error running the query [' . $db->error . ']'); 
    # this should be else*if*
    }else($result->num_rows==1){ 
    # but personally I'd change the whole approach:  
    #    if( num rows === 1){ good }elseif( error ){ ugly }else{ bad }
        $expires = 1 * 1000 * 60 * 60 * 24 * 2; 
        setcookie("username", $username, time()+$expires); 
        # why two cookies?
        # WHY SAVE THE HASH IN A COOKIE?  (bad security!)
        setcookie("password", $password, time()+$expires); 
        echo '<strong>Welcome,'.$username.'!</strong>';  
    }; 
    $mysqli->close();

[JonsAdventure]

Thanks again traq, so the current php file looks like this:

<?  
    $mysqli = new mysqli("");
    if ($mysqli->connect_errno > 0) {  
        die('Unable to connect to database [' . $mysqli->connect_error . ']');  
    }  
    $username = $mysqli->real_escape_string($_POST['user']);  
    $password = md5($_POST['password']);  
    $sql = <<< SQL 
SELECT * FROM `Users` WHERE `Username` = '$username' && `Password` = '$password' 
SQL 
; 
    if($result->num rows === 1){ 
        $expires = 1 * 1000 * 60 * 60 * 24 * 2;  
        setcookie("username", $username, time()+$expires);  
        echo '<strong>Welcome,'.$username.'!</strong>';
    }elseif( !$result = $db->query($sql) ){ 
        die('There was an error running the query [' . $db->error . ']');  
    }else{ 
        die('Damn, something messed up pretty badly.');  
    }  
    $mysqli->close();
?>

The html code which calls the function:

<form id="Process" name="Process" method="POST" action="Login/index.php">
	<fieldset>
		<label for="name" id="name_label">Username:</label>
		<input type="text" name="name" id="name" size="30" value="" class="text-input" />
		<label class="error" for="name" id="name_error">This field is required.</label><br/>
		<label for="password" id="password_label">Password:</label>
		<input type="password" name="password" id="password" size="30" value="" class="text-input" />
		<label class="error" for="password" id="password_error">This field is required.</label>
		<label class="error" id="login_error">Incorrect password or username.</label>
		<input type="submit" name="submit" class="button" id="submit_btn" value="Login" style="position:absolute; left:650px; top:0px;"/>
	</fieldset>
</form>

Oh, and I'm running php 5.3/.

[JonsAdventure]

The website is now working properly (don't use ie), but I'm having the same problem as before, the form submit button opens the file instead of running it.
http://www.thefunnyzone.co.uk

[traq]

if( !$result ){
    /* mysql error */
}elseif( $result->num_rows === 1 ){
    /* login successful */
}else{
    /* no matching username/password found */
} 

sorry for the confusion.

[Ashley Sheridan]

The website is now working properly (don't use ie), but I'm having the same problem as before, the form submit button opens the file instead of running it.
http://www.thefunnyzone.co.uk

The login form on that URL is just redirecting to a blank index.php file. Either you've got an error that's set to not display, or it really is blank. Nothing seems to be opening in a new tab though.