President Obama Posted January 11, 2011 Share Posted January 11, 2011 I tried Googling them and what not but all I could find was useless stuff that I couldn't get to work, so I thought I would give it a crack at making my own. I don't think its that secure though. Can someone have a geeza over it? I've pretty much made it up from bits and pieces I have seen and researched. Ignore the echoes they were just for testing. Well the code was working, now it just keeps redirecting me to index. So I dunno what I fucked. Heres all the code: Index.php <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd"> <html xmlns="http://www.w3.org/1999/xhtml"> <head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8" /> <title>Untitled Document</title> </head> <body> <?php include 'functions.php'; Connect(); ?> <form method="post" action="login.php"> <input type="text" name="Username" /> <input type="password" name="Password" /> <input type="hidden" name="ip" value="<?php ipget(); ?>" /> <input type="submit" /> </form> </body> </html> Login.php <?php require_once 'standalone\HTMLPurifier.standalone.php'; include "functions.php"; Connect(); $purifier = new HTMLPurifier(); $result = mysql_query("SELECT Username, Password FROM login ") or die(mysql_error()); $sorted = mysql_fetch_array($result); $name = $purifier->purify(strtolower($_POST['Username'])); $pass = $purifier->purify(md5(strtolower($_POST['Password']))); $ip = md5($_POST['ip']); $stamp = date("Ymdhis"); if ( $name == $sorted['Username'] ){ Echo "Username Correct"; if ( $pass == $sorted['Password'] ) { echo "Password is correct"; session_start(); $_SESSION['ip'] = $ip; $_SESSION['Username'] = $name; $_SESSION['Password'] = $pass; setcookie('ip', $ip, time()+3600); setcookie('name', $name, time()+3600); $ipb = $_SERVER['REMOTE_ADDR']; $orderid = "$stamp-$ipb"; $orderid = str_replace(".", "", "$orderid"); $GUID = md5(orderid); setcookie('GUID', $GUID, time()+3600); mysql_query("UPDATE login SET GUID = $GUID WHERE Username = '$name'"); header("location: admin.php"); } else { echo "password is wrong"; } } else { Echo "wrong name"; } ?> Functions.php <?php function connect(){ mysql_connect("localhost", "test", "password") or die(mysql_error()); mysql_select_db("db344475103") or die(mysql_error()); echo "Connected"; } function ipget(){ $ip = $_SERVER['REMOTE_ADDR']; echo $ip; } function check(){ session_start(); if (md5($_SERVER['REMOTE_ADDR']) == $_SESSION['ip']) { if (md5($_SERVER['REMOTE_ADDR']) == $_COOKIE['ip']) { if ($_SESSION['Username'] == $_COOKIE['name']) { if ($_COOKIE['GUID'] == mysql_query("SELECT GUID FROM login")) { } else { header("location: index.php"); session_destroy(); } } else { header("location: index.php"); session_destroy(); } } else { header("location: index.php"); session_destroy(); } } else { header("location: index.php"); session_destroy(); } } function clean(){ } ?> Admin.php <?php include 'functions.php'; check(); ?> <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd"> <html xmlns="http://www.w3.org/1999/xhtml"> <head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8" /> <title>Untitled Document</title> </head> <body> Admin Area </body> </html> Yeah its a lot of code, probably most of it useless as well knowing me. Quote Link to comment Share on other sites More sharing options...
colleyboy Posted January 11, 2011 Share Posted January 11, 2011 You only have else to point to index.php there is no command saying if the username and password are correct to actually point towards admin.php Quote Link to comment Share on other sites More sharing options...
President Obama Posted January 11, 2011 Author Share Posted January 11, 2011 Lol. I make the most silliest mistakes. Anyway security wise is it reasonable? Quote Link to comment Share on other sites More sharing options...
colleyboy Posted January 11, 2011 Share Posted January 11, 2011 Not a problem. Seems fine to me Quote Link to comment Share on other sites More sharing options...
phil88 Posted January 11, 2011 Share Posted January 11, 2011 There's a couple of little mistakes that I'm sure you'll notice when you come to running the code, for example; $GUID = md5(orderid) in login.php. As far as security goes, the general idea seems pretty good. The problem is that sessions are usually maintained using cookies. Therefore, hijacking a user's session cookie as well as the cookies you explicitly set would mean a hacker would then only need to worry about faking the contents of $_SERVER['REMOTE_ADDR'] so it matches the $_SESSION['ip'] - which depending on the circumstances could be pretty easy. It depends really on what you're trying to secure. If you used only HTTPS to set and get the session cookie it'd be more secure - but that might be overkill for what you're trying to achieve. Quote Link to comment Share on other sites More sharing options...
President Obama Posted January 11, 2011 Author Share Posted January 11, 2011 Its only for keeping people out of an admin area on a clan website, but I do want to go into the web design business so I want to try and get it strong as possible just to increase my knowledge. With the GUID you mean I left out the quotes didn't it? I got really lost on that. When I looked it up, the impression I got was to store it in a cookie and in a database then compare that cookie with the database which still seemed insecure to me if a hacker got the cookies. I thought by checking the ip in 2 ways it would be harder, because the user ip has to be the same as the cookie and session, so a hacker would have to hijack the session and steal the cookie but then I read somewhere that $_SERVER['REMOTE_ADDR'] was pretty useless at keeping things secure. I'm open to suggestions or ideas on how to make it more secure. Most of this I thought of on my own accord, except for the GUID bit, that just was confusing. Also any suggestions on making clear efficient code? I just can't help but think its a bit dirty and messy. Quote Link to comment Share on other sites More sharing options...
phil88 Posted January 11, 2011 Share Posted January 11, 2011 The point I made about the $GUID was just that; $GUID = md5(orderid) Will actually md5 the constant orderid, or the string 'orderid' if the constant doesn't exist (which it probably doesn't). If it tries to use the string, PHP will throw up a PHP Notice - which may or may not be displayed depending on your server config (I think they're hidden by default). What you were probably trying to do was; The point I made about the $GUID was just that; $GUID = md5($orderid) Probably just a typo (: It's a good idea to enable the output of PHP Notices in your development environment to help catch little mistakes like this. Using HTTPS rather than HTTP to post the initial login form and to set/retrieve cookies will help reduce the chance of someone stealing cookies or login details mid-transfer. You can use the secure parameter of PHP's setcookie to make sure cookies can only be retrieved over HTTPS. The security of other parts of the website could also be a factor in stealing cookies. If a hacker is able to submit javascript to your website and have it display to other users (ie, a forum or blog comment etc) then they could steal people's cookies using that javascript. The httponly parameter of PHP's setcookie can help minimise how effective that is, but as the manual states - not all browsers support it. So it's not going to remove the possibility completely. The long and short is; you can't make a 100% secure system. The methods you've done are likely to be enough for a clan website. Anything involving money or sensitive information should be using SSL to add another layer of security - but even that isn't bullet proof. For absolute security sensitive applications, assume your system will be broken and limit the damage that can be caused. Expire sessions after a few minutes so that stolen sessions cannot be used for very long. Log absolutely every action that required the user to be authenticated - it makes it easier to undo any damage once it's been done. This XKCD comic springs to mind. If someone wants access, they can get it regardless of how secure you think it is. Quote Link to comment Share on other sites More sharing options...
President Obama Posted January 12, 2011 Author Share Posted January 12, 2011 Jesus christ, I'm getting blinder by the day. You gotta pay to use HTTPS though right? Its nice to know that I built a script from the ground up that is secure enough. Ill check if that php notices thing is on in wamp. Thanks. I like this forum, I think I'll stick around. Quote Link to comment Share on other sites More sharing options...
phil88 Posted January 12, 2011 Share Posted January 12, 2011 Yeah, you have to pay to get a certificate from a trusted authority - or you could sign a certificate yourself for free...but browsers will display a big ugly warning when viewing your page telling users that the connection is encrypted but the browser can't necessarily trust identity of your website. Quote Link to comment Share on other sites More sharing options...
President Obama Posted January 12, 2011 Author Share Posted January 12, 2011 Oh righto. I won't worry about it then. Does encrypting the ip when its stored in the cookie and session help security much? Quote Link to comment Share on other sites More sharing options...
President Obama Posted January 12, 2011 Author Share Posted January 12, 2011 Sorry for double post but seeming I can't edit my other post... This code was working fine before I added the GUID but now I can't get it to work at all. Its as if the session/cookies wont start/set. I've tried removing the GUID part and parts that I didn't think were important to no avail. I really don't know what the problem is. Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.