PHP help needed

Masterpyro · December 11, 2006

Hey, im remodeling my site, http://slashwebfilters.isgreat.org and i need a few people to help me out with the login script. i am getting a few different errors about headers being sent twice or something like that. here is my code:

Login Form:

&lt;tr&gt;&lt;td&gt;

&lt;form action="login.php" method="post"&gt;

&lt;p&gt;

User ID:&lt;/p&gt;&lt;input type="text" name="userid" id="userid" /&gt;

&lt;/td&gt;

&lt;/tr&gt;

&lt;tr&gt;

&lt;td&gt;&lt;p&gt;

Password:&lt;/p&gt; &lt;input type="password" name="pass" id="pass" /&gt;

&lt;/td&gt;

&lt;/tr&gt;

&lt;tr&gt;

&lt;td&gt;

&lt;input type="submit" name="submit" id="submit" value="Submit" /&gt;

&lt;/td&gt;

&lt;/tr&gt;

&lt;/form&gt;

Login.php

&lt;?php

$host="***************"; // Host name 

$username="***************"; // Mysql username 

$password="*****************"; // Mysql password 

$db_name="********************"; // Database name 

$tbl_name="*****************"; // Table name 



// Connect to server and select databse.

mysql_connect("$host", "$username", "$password")or die("cannot connect"); 

mysql_select_db("$db_name")or die("cannot select DB");



// username and password sent from signup form 

$userid=$_POST['userid']; 

$pass=$_POST['pass']; 

$encrypted_pass=md5($pass);



$sql="SELECT * FROM $tbl_name WHERE userid='$userid' and pass='$encrypted_pass'";

$result=mysql_query($sql);



// Mysql_num_row is counting table row

$count=mysql_num_rows($result);

// If result matched $userid and $pass, table row must be 1 row



if($count==1){

// Register $userid, $pass and redirect to file "members.php"

session_register('$userid');

session_register('$encrypted_pass');

header("location:members.php");

}

else {

echo "&lt;p&gt;Invalid UserID or Password.&lt;/p&gt;";

}

?&gt;

Logout.php

&lt;? 

session_start();

session_destroy();

header("Location:index.php");

?&gt;

Register.php

&lt;?php 

// Connects to your Database 

mysql_connect("*********", "*************", "**************") or die(mysql_error()); 

mysql_select_db("****************") or die(mysql_error()); 



//This code runs if the form has been submitted

if (isset($_POST['submit'])) { 



//This makes sure they did not leave any fields blank

if (!$_POST['userid'] | !$_POST['pass'] | !$_POST['pass2'] | !$_POST['email']) {

die('&lt;p&gt;You did not complete all of the required fields&lt;/p&gt;');

}



// checks if the username is in use

if (!get_magic_quotes_gpc()) {

$_POST['userid'] = addslashes($_POST['userid']);

}

$usercheck = $_POST['userid'];

$check = mysql_query("SELECT userid FROM users WHERE userid = '$usercheck'") 

or die(mysql_error());

$check2 = mysql_num_rows($check);



//if the name exists it gives an error

if ($check2 != 0) {

die('&lt;p&gt;Sorry, the userid '.$_POST['userid'].' is already in use.&lt;/p&gt;');

}



// this makes sure both passwords entered match

if ($_POST['pass'] != $_POST['pass2']) {

die('&lt;p&gt;Your passwords did not match.&lt;/p&gt;');

}



// here we encrypt the password and add slashes if needed

$_POST['pass'] = md5($_POST['pass']);

if (!get_magic_quotes_gpc()) {

$_POST['pass'] = addslashes($_POST['pass']);

$_POST['userid'] = addslashes($_POST['userid']);

$_POST['email'] = addslashes($_POST['email']);

}



// now we insert it into the database

$insert = "INSERT INTO users (id, userid, pass, email)

VALUES ('".$_POST['id']."','".$_POST['userid']."', '".$_POST['pass']."', '".$_POST['email']."')";

$add_member = mysql_query($insert);

?&gt;



&lt;h1&gt;Registered&lt;/h1&gt;

&lt;p&gt;Thank you, you have registered - you may now &lt;a href="index.php" target="_parent"&gt;Login&lt;/a&gt;.&lt;/p&gt;

&lt;?php 

} 

else 

{    

?&gt;

&lt;form action="&lt;?php echo $_SERVER['PHP_SELF']; ?&gt;" method="post"&gt;

&lt;table align="center"&gt;

&lt;tr&gt;&lt;td&gt;&lt;p&gt;Username:&lt;/p&gt;&lt;/td&gt;

&lt;td&gt;

&lt;input type="text" name="userid" maxlength="60" /&gt;

&lt;/td&gt;&lt;/tr&gt;

&lt;tr&gt;&lt;td&gt;&lt;p&gt;Password:&lt;/p&gt;&lt;/td&gt;

&lt;td&gt;

&lt;input type="password" name="pass" id="pass" maxlength="30" /&gt;

&lt;/td&gt;&lt;/tr&gt;

&lt;tr&gt;&lt;td&gt;&lt;p&gt;Confirm Password:&lt;/p&gt;&lt;/td&gt;

&lt;td&gt;

&lt;input type="password" name="pass2" id="pass2" maxlength="30" /&gt;

&lt;/td&gt;&lt;/tr&gt;

&lt;tr&gt;&lt;td&gt;&lt;p&gt;Email:&lt;/p&gt;&lt;/td&gt;

&lt;td&gt;

&lt;input type="text" name="email" id="email" maxlength="100" /&gt;

&lt;/td&gt;

&lt;/tr&gt;

&lt;tr&gt;

&lt;td&gt;

&lt;input type="hidden" name="id" id="id" value="" /&gt;

&lt;/td&gt;

&lt;/tr&gt;

&lt;tr&gt;&lt;td&gt;&lt;input type="submit" name="submit" id="submit" value="Register"&gt;&lt;/td&gt;

&lt;td&gt;&lt;input type="reset" name="Reset" id="Reset" value="Reset"&gt;&lt;/table&gt;

&lt;/form&gt;



&lt;?php

} 

?&gt;

Members.php

&lt;? 

session_start();

if(!session_is_registered(userid)){

header("location:index.php");

}

?&gt;

Of course this is only the php code on the page and there will be more content eventually.

Is there anything happening on one page that isnt on another like register is encrypting password and login is not decrypting?

Any and all help appriciated.

omega_ion · December 11, 2006

Okay, your problem is that you are changing the header (location/url) after you have already begun outputting text, whether through echo or just closing your php tag and writing html.

Incorrect:

&lt;html&gt;

&lt;form name="login" action="login.php" method="POST'&gt;

etc....

&lt;/form&gt;

&lt;/html&gt;

&lt;?php

if($_POST[login]) {

   //do login sequence

  header("Location: index.php");

  }

?&gt;

The problem above is that you are changing the page after the current page has already been sent to the browser.

Correct:

&lt;?php

if($_POST[login]) {

   //do login sequence

  header("Location: index.php");

  }

?&gt;

&lt;html&gt;

&lt;form name="login" action="login.php" method="POST'&gt;

etc....

&lt;/form&gt;

&lt;/html&gt;

Notice how the header function comes before anything is sent to the browser.

cooper · December 11, 2006

With the "Headers already sent" thing, that basically means you already output some data before calling the header() function.

Make ABSOLUTELY sure the php files that use this function start with "<?" and end in "?>" with nothing in between that may cause data to be spat out on a path that isn't really supposed to.

Some other things to consider:

- What happens when a single quote is present in the username?

- NEVER, EVER use 'select *' in code. The order of the columns isn't defined in the database. If you decide to, for instance, rename a column by creating a new one with the new name, copying over the data and then dropping the original column, you're fucked. Also, in 99.9% of cases, you don't need all that data. Case in point, the select in login.php. All it wants to know is if the record exists. So what you should do is "select count('x') ...." and see if the end result is 0 or 1, and error out in all other circumstances.

- DON'T put the encrypted password on the session. What possible use does it have? All it does is take up space and open you up for attack.

- DON'T put the unencrypted password in a local variable. Treat the thing like the GPL at Microsoft: unless there is no escaping it, don't touch!

- I don't think putting the tablename in a local variable (or even a constant) makes good sense. It detracts from the readability of the script, all for the benefit that if you decide your DB is fucked (which at this point shouldn't be possible anyways. You DID put together a decent database before you started coding did you not?) you can use the same script with a differently named table with little effort. Use the find/replace function of your editor to deal with this eventuality.

- DON'T use session_register. As of PHP 4.1.0 the preferred way is $_SESSION['NAME'] = "value";

- It's good practice to have some files contain only logic, and others only deal with presentation. So in login.php when there is a problem, don't display an error from within this page. Using the header method redirect the user to a nice error page, passing along an explanation of the issue at hand. You can then make a single error page and reuse it throughout the site.

- Don't use "or die(error)". Redirect the user to a clean error page using the header() method.

- mysql_connect returns a resource. Hold on to it and use it. Don't assume it's used by default.

- mysql_select_db can and should be passed the resource that mysql_connect returned.

- Using a single pipe symbol means bitwise or. You want logical or, which is two pipe symbols.

- By saying "!$value" you're basically only comparing it to not being set. You'll probably want to check for length and do other things, so your method should be expanded a bit.

- Write a dedicated method for checking if a user already exists. Will save you from having to come up with stupic variable names like 'check' and 'check2' rather than the more sensible (give the context) 'dbresult' and 'rowcount'.

- Fail fast. Perform easy checks that are cheap on resources first. So check for matching passwords before going to the DB to see if the username's valid.

- The md5 method returns a HEX string. No need for addslashes on that one.

- Use mysql_real_escape_string to escape strings. The addslashes method is generic and primarily geared towards quotes. There are other characters in DB and shell land that have special meaning.

- Use the sprintf method to assemble long and/or complex query strings. It's much tidyer.

- I think all your pages should start with session_start()...

Masterpyro · December 11, 2006

ok thanks guys that sounds like it will solve my problem. i will impliment the changes a little later, im starting my pure pwnage marithon :)

Masterpyro · December 14, 2006

ok i got one more problem. users can register, but they cant login...still. No more header errors just an infinite loop or something somewhere. here is my code:

Login

//Checks if there is a login cookie

if(isset($_COOKIE['ID_my_site']))



//if there is, it logs you in and directes you to the members page

{ 

$userid = $_COOKIE['ID_my_site']; 

$pass = $_COOKIE['Key_my_site'];

$check = mysql_query("SELECT * FROM users WHERE userid = '$userid'") or header("Location: error.html");



while($info = mysql_fetch_array( $check )) 

{

if ($pass != $info['pass']) 

{

}

else

{

header("Location: members.php");



}

}

}



//if the login form is submitted

if (isset($_POST['submit'])) { // if form has been submitted



// makes sure they filled it in

if(!$_POST['userid'] | !$_POST['pass']) {

header("Location: error.html");

}

// checks it against the database



if (!get_magic_quotes_gpc()) {

$_POST['userid'] = addslashes($_POST['userid']);

}

$check = mysql_query("SELECT * FROM users WHERE userid = '".$_POST['userid']."'") or header("Location: error.html");



//Gives error if user dosen't exist

$check2 = mysql_num_rows($check);

if ($check2 == 0) {

header("Location: error.html");

}

while($info = mysql_fetch_array( $check )) 

{

$_POST['pass'] = stripslashes($_POST['pass']);

$info['pass'] = stripslashes($info['pass']);

$_POST['pass'] = md5($_POST['pass']);



//gives error if the password is wrong

if ($_POST['pass'] != $info['pass']) {

header("Location: error.html");

}

else 

{ 



// if login is ok then we add a cookie 

$_POST['userid'] = stripslashes($_POST['userid']); 

$hour = time() + 3600; 

setcookie(ID_my_site, $_POST['userid'], $hour); 

setcookie(Key_my_site, $_POST['pass'], $hour); 





//then redirect them to the members area 

header("Location: members.php");

} 

} 

} 

else 

{ 



// if they are not logged in 

?&gt;

i have no idea what is wrong with it still but i think it is at the setcookie() functions at the bottem of the code, because if a wrong pass is entered it goes to my error page but if correct pass is given it goes into some kinda infinite loop as firefox says.

Sign up for our new forums @ http://slashwebfilters.phpnet.us/forums (this login actually works!)

cooper · December 14, 2006

Believe it or not, when I reply to someone's post, I tend to invest time and effort in conveying a message. Here's the link in case your scrolll wheel is broken.

I pointed out a number of security issues in your code, and from the looks of it you've fixed none of them.

You might want to work on the problems that have been pointed out already before you try and 'enhance' the project.

Masterpyro · December 14, 2006

o ya sorry i fogot that i only did this with the register page.

Deveant · December 14, 2006

T_T sorry to say this but i have to agree with cooper, the security in ur scripts, are umm, weak at most. Though ive been browsing through ur site, and have checked out a few things, and seeing as though u have a PHPBB forum, why not use it?

Having a look at ur scripts it seems as though either u are limited in ur PHP knowledge our u are totaly new to is, so the biggest thing i can suggest to any new comers of pretty much any code language, "there is no need to re-invent the wheel - 'Unless its hello world'", pretty much what ur doing, has pretty much been the focus of many web desingers, and now there are tons of good, high security scripts out there, with great HOWTO's and such.

Though as mentioned why not utalise ur PHPBB? im talking bout Website Integration, (cross referencing of a Site and Forum) Over a few years of working for web sites, ive compiled a rather user friendly and security friendly libarary of Website Integration Scripts for PHPBB. If u wanna copy then feel free to IM me, and ill send em out to u.

Also the issue with ur banner having the blank space is caused by ur html, try changing:

&lt;table align="center" border="2" width="850" height="1420"&gt;

to:

&lt;table align="center" border="2" width="850" height="100%"&gt;

Okay if u have questions, need help or want the scripts feel free to IM me ^_^

(also what cooper said, should really be taken in, for a learning curve, what he says is not only a great way in improving ur script, it follows best practce of PHP)

Masterpyro · December 14, 2006

yes i am new to php, i got this code from about.com and attempted to edit accordingly. i was not sure if i could connect the site with the forums but that is what i really wanted to do. I am not sure but basically all i have to do is have the site look for the same cookie the forums create? if thats all i will do it that way.

cooper · December 14, 2006

I think cookies are tied to the domain. So unless your code is placed on the same server, this will not work (and shouldn't work).

Example:

My bank stores my username for their telebanking thing inside a cookie on my machine (because I told it to. Off by default). I don't want some yahoo that happens to be granted the privilege that is my humble presence to be able to know that that is my username on that site. If what you wanted were possible, that yahoo could be doing this aswell. And people wouldn't be using the internet for banking anymore.

Masterpyro · December 14, 2006

well if i could get it to check the cookies with the database of the forums wouldnt it be possible?

its on the same domain

Deveant · December 15, 2006

the codes dont relly on being in the same server, though if it is not on the same server then u will need to open up ur SQL to be X'ed with other sites (so 'a' can access the SQL on 'b'), All the cookies data and such for most forums are stored in its DB (well for PHPBB, IPB, and Post Bulleten), so what the script do is to access the info and work in the pretty much same way as what the forums do only with out all the forums HTML, and such. By default the codes use existing pecies of the forum for config (security reasons) though u can make ur own conf files of ur forum is off site.

Masterpyro · December 15, 2006

so i could just have the page compare with the forum DB right?

Deveant · December 15, 2006

yeah thats pretty much how it works.

Sign In

PHP help needed

Recommended Posts

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Join the conversation

Recently Browsing 0 members