3raser Posted March 1, 2012 Share Posted March 1, 2012 I had a spam bot attacking on one of my forums. I quickly wrote a feature that allows me to delete all posts, threads, and IPs (and any other account they have) via username. In my create.php page, you need to be logged in ($_COOKIE['user']) in-order to post. How is it that they posted without being logged in? Quote Link to comment Share on other sites More sharing options...
PFMaBiSmAd Posted March 1, 2012 Share Posted March 1, 2012 How do you know they weren't logged in? Also, if you are just testing for the existence of a cookie or even a cookie with a publicly displayed username in it for a value, anyone can send a cookie with any value when they make a http request for pages on your site. Does your log in check code have any logic to prevent the remainder of the code on the page from running if someone isn't logged in? Does your form processing code check if someone is logged in before processing the form data? It would take actually seeing your code to pin down if or how someone posted to it without being logged in. Quote Link to comment Share on other sites More sharing options...
3raser Posted March 2, 2012 Author Share Posted March 2, 2012 Here: http://pastebin.com/iiLMCHq2 Sorry it's a little sloppy. But I'm pretty sure I pass all the correct tests before actually displaying the form data. Test for yourself here: http://www.osremake.org/forums/addreply.php Quote Link to comment Share on other sites More sharing options...
Pikachu2000 Posted March 2, 2012 Share Posted March 2, 2012 What's in your redirect() function? Quote Link to comment Share on other sites More sharing options...
PFMaBiSmAd Posted March 2, 2012 Share Posted March 2, 2012 I suggest you fix the following flaw in your login code, ASAP - you are just testing for the existence of a cookie or even a cookie with a publicly displayed username in it for a value, anyone can send a cookie with any value when they make a http request for pages on your site. You also didn't answer the question I asked. Quote Link to comment Share on other sites More sharing options...
PFMaBiSmAd Posted March 2, 2012 Share Posted March 2, 2012 If you search this forum, under my username, for the keywords - 'unique hard guess', you well get a number of posts describing what you should be storing in a 'login/remember me' cookie, then using the unique and hard to guess value in the cookie only to identify the visitor to find his user information in your user table, and then determining the logged in/out status based on a value stored solely on the server, and not based on the simple existence of a cookie. Quote Link to comment Share on other sites More sharing options...
xylex Posted March 2, 2012 Share Posted March 2, 2012 And as PFM points out, cookies aren't to be trusted, so you have a SQL injection spots here "SELECT `lastpost` FROM `users` WHERE `username` = '{$_COOKIE['user']}'" And here "INSERT INTO `threads` VALUES (null, '$id','$title', '$content', '{$_COOKIE['user']}', NOW(), '". qfc() ."', NOW(), '{$_COOKIE['user']}', '','{$_SERVER['REMOTE_ADDR']}', '0', '0', '$s', '')" The latter of which could easily be used to spam your database. Quote Link to comment Share on other sites More sharing options...
PFMaBiSmAd Posted March 2, 2012 Share Posted March 2, 2012 Here's a slightly different slant on the question I think you tried to ask. I think your first post in this thread was actually trying to ask - You have banned/removed an account, but that person (or bot script) is still able to post replies in your forum. The answer to that has already been given. Your login check code simply tests for the existence of a cookie. As long as that cookie is sent along with the request for your form processing code, your code testing if that cookie isset will allow anyone to submit to the form processing code. The only thing your code is testing based on the value in that cookie is how many characters are permitted. P.S. References to your redirect() function currently result in a fatal undefined function error message on your site. P.P.S. When you ban an account, you should simply mark it as being banned and when you delete a post, you should simply mark it as being deleted (in real world applications, data is almost never actually deleted.) When you actually delete this information, you lose the record of the username, email address, and ip address/ip history. Quote Link to comment Share on other sites More sharing options...
3raser Posted March 4, 2012 Author Share Posted March 4, 2012 What's in your redirect() function? function redirect($url) { header('Location: '. $url); } I'm assuming you're thinking it's XSS vulnerable? I suggest you fix the following flaw in your login code, ASAP - you are just testing for the existence of a cookie or even a cookie with a publicly displayed username in it for a value, anyone can send a cookie with any value when they make a http request for pages on your site. You also didn't answer the question I asked. I'm assuming hashing it then adding some simple form of salt would help secure it? And to answer your question: If someone isn't logged in while viewing the page, the code shows them the form. In some cases around the website, if they aren't logged in, it'll immediately redirect them to another page, thus ending the process of the code (suppose to, anyways). And yes, most forms have a small line of verification to make sure they're logged in before it continues processing the code. Usually like so: if(isset($_COOKIE['user'])) { //process } else { //display error message or redirect them somewhere else } Some lines of verification are like below, but I'm not sure if makes a difference than the one above. if(!isset($_COOKIE['user'])) redirect('index.php'); //code If you search this forum, under my username, for the keywords - 'unique hard guess', you well get a number of posts describing what you should be storing in a 'login/remember me' cookie, then using the unique and hard to guess value in the cookie only to identify the visitor to find his user information in your user table, and then determining the logged in/out status based on a value stored solely on the server, and not based on the simple existence of a cookie. Thanks, I'll go check it out soon as I finish up this reply. And as PFM points out, cookies aren't to be trusted, so you have a SQL injection spots here "SELECT `lastpost` FROM `users` WHERE `username` = '{$_COOKIE['user']}'" And here "INSERT INTO `threads` VALUES (null, '$id','$title', '$content', '{$_COOKIE['user']}', NOW(), '". qfc() ."', NOW(), '{$_COOKIE['user']}', '','{$_SERVER['REMOTE_ADDR']}', '0', '0', '$s', '')" The latter of which could easily be used to spam your database. Wow, I've never really thought about that. I guess someone could easily edit the cookie to some SQL injection code and the page will try to process it as such. Would you recommend me using a whole different method or just escaping the cookie with mysql_real_escape_string()? Here's a slightly different slant on the question I think you tried to ask. I think your first post in this thread was actually trying to ask - You have banned/removed an account, but that person (or bot script) is still able to post replies in your forum. This wasn't really my question, but since you said that - I'm sure it could happen (with all the reasons you guys have listed in the above posts). The answer to that has already been given. Your login check code simply tests for the existence of a cookie. As long as that cookie is sent along with the request for your form processing code, your code testing if that cookie isset will allow anyone to submit to the form processing code. The only thing your code is testing based on the value in that cookie is how many characters are permitted. I guess I never really thought that out. I could always take the cookie, then run a username check through the database. But it all honestly depends on how I plan on changing the system. And thanks to xylex for pointing out the fact it leads to a possibility of having SQL injection attacks. P.S. References to your redirect() function currently result in a fatal undefined function error message on your site. Which page is having the error, or is it global issue around the site for you? I haven't seen any redirect failures. P.P.S. When you ban an account, you should simply mark it as being banned and when you delete a post, you should simply mark it as being deleted (in real world applications, data is almost never actually deleted.) When you actually delete this information, you lose the record of the username, email address, and ip address/ip history. One of the fileds in the "users" table is "banned". Whenever a user is banned, I change the 1 -> 0. Whenever they attempt to login (will logout them out if they're logged in, too) it basically runs a check with the database to make sure they aren't banned (checking if the field "banned" is a 0). Although, in this case, deleting all the accounts was very necessary. They spammed the registration system with thousands of accounts (I added in a system to only allow 3 accounts per IP, so this should help avoid that) so I felt it was necessary to delete all of them. Luckily the user who initiated the attack made an account with an identical username and email of one of the bots, therefor allowing me to get his actual IP adress. The account is still in the database, although banned. And to respond to deleting posts, it's basically the same exact way. Instead of deleting, I "hide" posts. I appreciate the replies, A LOT. This is very helpful and I appreciate you guys taking your time to help me out. Quote Link to comment Share on other sites More sharing options...
Pikachu2000 Posted March 4, 2012 Share Posted March 4, 2012 What's in your redirect() function? function redirect($url) { header('Location: '. $url); } I'm assuming you're thinking it's XSS vulnerable? No I was assuming you didn't have an exit() following the header() redirect, allowing code to continue to execute after the header() is sent. If the browser ignores the header, the rest of the code still executes as though the redirect isn't even there. Quote Link to comment Share on other sites More sharing options...
PFMaBiSmAd Posted March 4, 2012 Share Posted March 4, 2012 When I reviewed your other recent threads, I'm pretty sure I saw one where Pikachu2000 stated to add an exit() after your header() redirect in that exact function. I'm guessing that after you tried that, you removed it. You need it for security purposes. It will also fix some types of unexplained problems with your code, by preventing the remainder of your code from running while the browser requests the new URL. Quote Link to comment Share on other sites More sharing options...
3raser Posted March 4, 2012 Author Share Posted March 4, 2012 What's in your redirect() function? function redirect($url) { header('Location: '. $url); } I'm assuming you're thinking it's XSS vulnerable? No I was assuming you didn't have an exit() following the header() redirect, allowing code to continue to execute after the header() is sent. If the browser ignores the header, the rest of the code still executes as though the redirect isn't even there. Ah, thanks. I'll add this immediately. When I reviewed your other recent threads, I'm pretty sure I saw one where Pikachu2000 stated to add an exit() after your header() redirect in that exact function. I'm guessing that after you tried that, you removed it. You need it for security purposes. It will also fix some types of unexplained problems with your code, by preventing the remainder of your code from running while the browser requests the new URL. Do you mean the post above? I haven't added the exit() function yet; this is the first time me seeing Pikachu suggesting it. I may have passed it up somewhere else if not here. And I see what you're saying. If the redirect function fails for some reason, they basically have access to something they aren't supposed to have access to. Thanks guys. Quote Link to comment Share on other sites More sharing options...
kicken Posted March 4, 2012 Share Posted March 4, 2012 Do you mean the post above? I haven't added the exit() function yet; this is the first time me seeing Pikachu suggesting it. I may have passed it up somewhere else if not here. And I see what you're saying. If the redirect function fails for some reason, they basically have access to something they aren't supposed to have access to. Doesn't really matter if the redirect "fails" or not, the code is still going to be run allowing them to do whatever they normally could without the redirect. I posted an example of why you need to exit after a redirect over on devshed. I'll re-post here, since DS appears to be having some issues lately (maybe just me). A lot of scripts handle access by including a 'secure.php' file which validates the user by session/posted username/passwords, and in the case of a login failure redirects to the login page. Remember to always do this redirect in such a way that the rest of the script is not executed after the redirect. The easiest way to do this is using exit; after the redirect. Example: I've been recently looking over code for a company to identify security holes and they were doing this for their authentication. they had code which boils down to something like this: <?php if (isset($_COOKIE['username']) && isset($_COOKIE['userpass'])){ $rs = mysql_query('SELECT password FROM users WHERE username="'.$_COOKIE['username'].'"'); if (mysql_num_rows($rs) == 0){ header('Location: login.php'); } else { $securepass=mysql_result($rs, 0, 0); if (base64_decode($_COOKIE['userpass']) != $securepass){ header('location: login.php'); } } } That file is used in an admin file as so: <?php //Validate login, redirect to login page if incorrect. include('secure.php'); $action = $_GET['action']; if ($action == 'delete'){ mysql_query('DELETE FROM entries WHERE entry_id='.$_GET['id']); echo 'Entry deleted'; } ?> Now. Aside from the glaring SQL Injection problems, it is also possible for a completely unauthenticated user to delete stuff because they do not prevent the rest of the script from running. A request to the URL http://www.site.com/admin/edit.php?action=delete&id=1 will generate a response like so: HTTP/1.1 200 Ok Location: login.php Entry Deleted The browser will happily redirect you to login.php and it looks like your code works, but if you look at the DB you'll be quick to notice entry_id #1 is no longer there because the delete was executed when the script finished up. The fix? Just exit after a redirect. header('Location: login.php'); exit; Quote Link to comment Share on other sites More sharing options...
3raser Posted March 4, 2012 Author Share Posted March 4, 2012 Ah, thanks for the reply kicken. Read everything. Quote Link to comment Share on other sites More sharing options...
3raser Posted March 4, 2012 Author Share Posted March 4, 2012 Please give me any feedback/ideas you have about this: This is how I plan on securing cookies. I made a new row/field in the users table called "cookie" that stores a random hashed & salted value upon creation of the account. This is what I use: //generate cookie $cookie_value = substr(sha1(md5($username.$e3).$e2).md5(sha1($username.$e3).sha1($e1)), 1, 30); function createCookie($username, $e1, $e2, $e3) { //generate cookie $cookie_value = sha1(md5($username.$e3).$e2).md5(sha1($username.$e3).sha1($e1)); //ignore this if(checkCookie($cookie_value)) { } else { } } Then I plan on storing that with the user. And whenever I do $lb->isLoggedIn() to check if a user is logged in, I run this piece of validation code: public function isLoggedIn() { //query $query = mysql_query("SELECT cookie FROM users WHERE username = '". mysql_real_escape_string($_COOKIE['user']) ."' LIMIT 1"); if(mysql_num_rows($query) < 1) { return 0; } else { //retrieve the stored cookie value $fetch = mysql_fetch_assoc($query); if($fetch['cookie'] !== $_COOKIE['user_session']) { return 0; //they shouldn't have the cookie of a user when they don't match $this->destroyCookie($_COOKIE['user_session']); } else { return 1; } } } Any ideas or feedback? Quote Link to comment Share on other sites More sharing options...
3raser Posted March 4, 2012 Author Share Posted March 4, 2012 Please ignore the post above! Can someone try and test if the cookies are vulnerable? (http://www.osremake.org) I spent the past two hours writing new validation code, changing the login system, and editing all the pages to the new functions to make it operational. I believe everything is secure now. You can no longer edit a cookie to any username you want, as the new code will actually check if the cookie exists in the database. If it does, it withdraws the username that cookie is matched to. The cookie value is also hashed and salted. I'd rather not release the way it's hashed and salted, but even if I did - part of it is based off of random numbers using the rand() function. 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.