Forum Moderators: coopster

Message Too Old, No Replies

Members area login

Creating a members area, but anyone can access

         

photomike

6:48 pm on Jan 21, 2011 (gmt 0)

10+ Year Member



I have created a login form and coded the php file to log in the member. I have coded each page of the member's area to authenticate the user; however, anyone can still access the pages by simply typing in the URL. The codes are not returning any errors when run on the server.

Here is the code to process the login form:
<?php

$dbhost='myserver';
$dbuser='user';
$dbpass='pass';

$conn = mysql_connect($dbhost, $dbuser, $dbpass) or die ('Error connecting to mysql');

$dbname='databasename';
mysql_select_db($dbname);


session_start();
$username = $_POST['username'];
$password =md5($_POST['password']);
$sql = "select * from table where username='$username' and password='$password'";
$result = mysql_query($sql);
if (mysql_num_rows($result) !=1) {
$error = "Login failed";
} else {
$_SESSION['username'] = "$username";
$_SESSION['ip'] = $_SERVER['REMOTE_ADDR'];
header('Location: http://www.mydomain.com/' .$username. '.php');
}


?>


as you can see, each member will be directed to their own page through the header, and that works perfectly bringing up their gallery.

Here is the code I put at the top of all members pages:

<?php
session_start();
$newip = $_SERVER['REMOTE_ADDR'];
if (!isset($_SESSION['username']) ||
empty($_SESSION['username']) || $newip!= $_SESSION['ip']) {
include "logout.php";
}
?>


I have put this at the top of the page before the <html> tag, maybe that is my mistake; I am quite new at coding for members sections.

Also, as of right now, security is not a major concern as the only info stored in the db is the username and password and the members pages are galleries of pictures for the Bride and Groom and any guests they give their event password to (it's for a photography business); however, I do want to eventually expand the galleries to be able to take orders from page visitors, so I would appreciate if anyone could tell me of any major security issues I should deal with now, so I don't miss them at a later expanssion.

Thanks,
Mike

Tommybs

1:23 pm on Jan 26, 2011 (gmt 0)

10+ Year Member



Do you have an else statement around the content you show to logged in users? e.g if(not logged in){
//logoutformm
}else{
//show stuff here
}

if it's not in an else statement it will still show.

Also I'd do some data data cleansing on your login form

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

(and for username!)

omoutop

2:00 pm on Jan 26, 2011 (gmt 0)

WebmasterWorld Senior Member 10+ Year Member Top Contributors Of The Month



from what i see on your code:

If i make a login as "User1" i will be redirected in
http://www.mydomain.com/User1.php
.

Using "user1" as login, then we have
http://www.mydomain.com/user1.php.


Also i can type
http://www.mydomain.com/otheruser.php
on my browser and still be able to browse this page as i am legally logged, but i am not "otheruser".

Now, you are saying that this redirects to a gallery page.
So, i can visit ANY gallery and perform actions there? Like delete for example? Or messing with its tags?

Do you see the issues here?
Having simple
$_SESSION['username']
isn't enough.

Also, for the use of
mysql_real_escape_string(md5($_POST['password']));

The escape string is not needed in this case, data are altered by the use of md5().
The username on the other hand needs the use of the escape strings.

Last, on the top of your pahe have something like this:
echo '<pre>';
print_r($_SESSION);
print_r($_POST);
print_r($_GET);
echo '</pre>';


This will help you check what kind of data are being passed to this page. Chek your data and you will discover whats wrong with your login

rocknbil

5:41 pm on Jan 26, 2011 (gmt 0)

WebmasterWorld Senior Member 10+ Year Member



Here is your problem (and IMO, the problem with many PHP scripts I see out there . . . .)

header('Location: [mydomain.com...] .$username. '.php');
}

(s added just so you can see the code)

What this does is doubles your work, really, by requiring functions you've already done in this script. With each redirect you do, you have to re-validate the user. So all of the validation you've done here needs to be repeated in $username.php.

Some might argue that since you've set a session, you can use the session variable in $username.php, but it still has to be validated:

- Sessions are only maintained with the client (browser) via the PHPSESSID cookie, and can be spoofed.
- without a second lookup, what are you comparing against? It's entirely possible other users can log in and hack this user's account by just changing the user name.

Here is what I'd suggest:

include ('validation.php');

in all scripts requiring validation. validation.php would do something like

if ($username) { // session variable
$isvalid= validation_here($username);
}
if (! $isvalid) {
return_to_login_form();
// the login form explicitly EXITS after output
}
// Otherwise, it continues on, no "else" required

That's it . . . you would have one self contained include that either validates the user and lets the rest of the current script run or it outputs a login form. This allows you to continue using redirects and multiple scripts as you're used to with minimal redesign.

if anyone could tell me of any major security issues I should deal with now


Never directly use uncleansed input in your scripts.
$username = $_POST['username'];
$password =md5($_POST['password'])

At a bare minimum,
$username = mysql_real_escape_string(strip_tags($_POST['username']));
$password = md5(mysql_real_escape_string(strip_tags($_POST['password'])));