/    Sign up×
Community /Pin to ProfileBookmark

Login help, can someone test it for me

I have set up a login form, not sure how secure it is, the code is below. Is this the most secure way to do this or is there something I can do better.. the code is in the reply below.

Thanks

to post a comment
PHP

11 Comments(s)

Copy linkTweet thisAlerts:
@cjc1055authorFeb 02.2007 — To clarify.. what I'm trying to make sure is that no one can view the pages past the login, because I am planning to house information past that login which I don't want anyone to be able to view other than registered users.

I am new to sessions so I want to make sure this site isn't hackable so people won't be able to bypass the login.

Thanks,

Chris
Copy linkTweet thisAlerts:
@bokehFeb 02.2007 — It would be helpful if you were to post your code if you are looking for security vulnerabilities, rather than expect people to waste their time trying to hack the site.
Copy linkTweet thisAlerts:
@cjc1055authorFeb 02.2007 — Sorry, didn't really think about that ?

what I am using is a validation page that checks the mysql database for the username entered and checks if the password matches, if it does

session_register('username');

then every page that requires authentication uses if (session_is_registered('username')) { show all page info; } else { you must be logged in... etc. }
Copy linkTweet thisAlerts:
@MatMelFeb 02.2007 — I have just put an ' in the username field and the page gave me some error.

You should be careful with mysql injection.

Ive tried some simple things to avoid the login but was not successful.

But Im far from an expert in mysql injection, so you should look if you can do something !
Copy linkTweet thisAlerts:
@bokehFeb 02.2007 — Without showing your code you are not going to get helpful advice.
Copy linkTweet thisAlerts:
@cjc1055authorFeb 02.2007 — [code=php]<?php session_start();
include 'header.php'; ?>
<tbody>
<tr>
<td width="50%" class="content"><p align="center"><img src="images/cab2_foto.gif" width="311" height="200" alt="Physician"><br>
</p>
</td><td width="49%" class="content">
<?php
if (isset($_POST['hidden']) == "hidden") {
include 'config.php';
if (isset($_POST['submit'])) {
$sql = "SELECT * FROM logintable WHERE username = '". $_POST['username'] ."'";
$sql = mysql_query($sql);
$result = mysql_fetch_assoc($sql);
if($_POST['username'] == $result['username'] &&
$_POST['password'] == $result['password']) {
print '<p>Login Successful</p>';
if ($result['adminprviledges'] == "Yes") {
echo "<a href="admin.php">Admin Home</a><br>";
echo "<a href="manageusers.php">Manage Users</a><br>";
echo "<a href="viewcontacts.php">View Contacts</a><br>";
echo "<a href="logout.php">Logoout</a>";
session_register('usernameadmin');
session_register('passwordadmin');
}
elseif ($result['adminprivledges'] == "No") {
echo "You are logged in!<br>";
echo "<a href="admin.php">Admin Home</a><br>";
echo "<a href="viewcontacts.php">View Contacts</a><br>";
echo "<a href="logout.php">Logout</a>";
session_register('username');
session_register('password');
}
}
else {
print '<p>Login Failed</p>';
echo "<p><a href="login.php">Try Again</a></p>";
}
}
}
?>
</td>
</tr>
</tbody>
</table>
</div>

</td>
</tr>
</tbody>
</table>
<?php include 'footer.php'; ?>[/code]
Copy linkTweet thisAlerts:
@cjc1055authorFeb 02.2007 — And this is on top of every other page....[code=php]<?php session_start(); ?>
<? if (session_is_registered(username) || session_is_registered(usernameadmin)) { show page } else { echo "login failed"; }
?>[/code]
Copy linkTweet thisAlerts:
@cjc1055authorFeb 02.2007 — MatMel, Thanks, I fixed that problem
Copy linkTweet thisAlerts:
@cjc1055authorFeb 03.2007 — Anyone have any advice on my code? I want to make sure I have it as secure as I can.

Thanks.
Copy linkTweet thisAlerts:
@NightShift58Feb 03.2007 — (1) To prevent SQL injections, do not use:[code=php] $sql = "SELECT * FROM logintable WHERE username = '". $_POST['username'] ."'"; [/code]Ensure that you POST variables are run through mysql_real_escape_string().

(2) It seems that the passwords in your table are stored in clear text. Not a good idea. You should encrypt them before you start sending them back and forth across the internet.

(3) Not a security issue, but after the query you are again checking if the user names match. It's a little redundant as you wouldn't have gotten this far if they didn't.

(4) You should limit your query to 1 row with "LIMIT 1". There should be not more than 1 user with that user name.

(5) You are using $sql for both the SQL statement string and the resource name. It's unusual but not dangerous.
Copy linkTweet thisAlerts:
@cjc1055authorFeb 03.2007 — Ok, I added this to above the sql statement: [code=php]$_POST['username'] = mysql_real_escape_string($_POST['username']); [/code]

I've also encrypted the passwords.

I also removed the second check if the usernames match.

Query was limited to one row.


Anything else, or so I seem to be ok now?
×

Success!

Help @cjc1055 spread the word by sharing this article on Twitter...

Tweet This
Sign in
Forgot password?
Sign in with TwitchSign in with GithubCreate Account
about: ({
version: 0.1.9 BETA 6.2,
whats_new: community page,
up_next: more Davinci•003 tasks,
coming_soon: events calendar,
social: @webDeveloperHQ
});

legal: ({
terms: of use,
privacy: policy
});
changelog: (
version: 0.1.9,
notes: added community page

version: 0.1.8,
notes: added Davinci•003

version: 0.1.7,
notes: upvote answers to bounties

version: 0.1.6,
notes: article editor refresh
)...
recent_tips: (
tipper: @meenaratha,
tipped: article
amount: 1000 SATS,

tipper: @meenaratha,
tipped: article
amount: 1000 SATS,

tipper: @AriseFacilitySolutions09,
tipped: article
amount: 1000 SATS,
)...