Jump to content

phil88

Members
  • Posts

    111
  • Joined

  • Last visited

    Never

Everything posted by phil88

  1. Firstly, congratulations on looking to better your skills. A lot of people get into a way of doing things and are too stubborn to change, even if the way they're doing it is awful. That being said, you're right in thinking that your code is vulnerable to security exploits The first one that stands out to me is this line of your login.php file; $query = "SELECT UserID FROM Users WHERE UserName = '$_POST[username]' AND UserPassword = '$_POST[password]'"; It's open to a SQL Injection attack, which could cause all kinds of problems from deleting or updating your tables, to silently stealing your data. The problem is that those $_POST values could contain absolutely anything. If we send admin';# as the username, for example, your query will become; $query = "SELECT UserID FROM Users WHERE UserName = 'admin';# AND UserPassword = '$_POST[password]'"; # is a MySQL comment, so everything after it will be ignored. The query will then return the 'admin' row regardless of the password entered. Read up on sanitizing the input values, preventing mysql injections and using prepared statements (Google will find hundreds of tutorials, there's no need me rewriting the wheel here). Another problem that stands out is your loginaction.php script. There's nothing to stop a user navigating to www.yourdomain.com/loginaction?php?UserId=1 (or any other arbitrary user id) and logging in as that user id. It would make more sense to have the session creation in login.php inside the $count == 1 if statement.
  2. Totally untested, but you could probably do something along this lines of: $page = substr(strrchr($_SERVER['HTTP_REFERER'], '/'), 1);
  3. The reason why your original code isn't doing what you want it to do is because this code; $page->js = "js/jquery-1.3.2.min.js"; $page->js = "js/revealClass.js"; Overwrites the $page->js property. It's the equivalent of doing; $var = 'string 1'; $var = 'string 2'; $var = 'string 3'; echo $var; Only the value that was last assigned to $var gets echo'ed. As Nightslyr said, using arrays would be more appropriate. To keep with the original idea of using the js property of your Page class instead of passing the scripts as a parameter, you could do something like this; class Page { public function display_js() { foreach($this->js as $script) { echo "<script type='text/javascript' src='$script'></script>\n"; } } } $page = new Page; $page->js = array("js/jquery-1.3.2.min.js", "js/revealClass.js");
  4. Yeah, we're on the same page about the background - creating a better background is something I'm going to do. My comment regarding my priorities was about creating a logo though - there's no marks in doing that, so that's a long way down the to-do list.
  5. Thanks for the feedback guys. I totally agree about the spacing of some elements and the background being blurry and a bit much. I've found that it doesn't notice too much in 1024x768 windows, but anything larger and it really does stand out. As far as the logo/header goes, this is a uni assignment, so unfortunately I'm prioritising the marks at the moment. Creating a logo is something that is definitely on my to-do list, but it's quite a long way down for now because there are bigger marks in other areas that I need to focus on. Did you both fill in the questionnaire?
  6. Filter out HTML on the server-side using PHP, don't just do it with JS. It is VERY easy to get around a filter if you just use JS because anyone can just turn JS off in their browser, and thus, turn off your filtering. Also, don't have their IP sent as a hidden form field. That can also be manipulated by a malicious user to contain anything they want very easily. Instead, just get the user's IP address in the page that handles the form submission. You can get the poster's IP address using $_SERVER['REMOTE_ADDR'] with having the modify your form at all.
  7. Are the cookies being set correctly? Put a var_dump($_COOKIES); in your index.php file and run through the process to see
  8. Hey guys, first I'd like to apologise for initially posting this in the wrong forum. I didn't find much distinction between this and the critique forums. Anyhoo. I've created a social network aggregator for my final year project at uni and would appreciate your time to help me test it. At the moment, it supports linking Twitter, Facebook & Myspace accounts together and letting you read and post updates from and to each account, but it's built with extendibility in mind. This is the website and I'd really appreciate if you would test it on your PCs, Macs, mobiles or whatever other devices you have. I'd also massively appreciate it if you'd fill in this questionnaire if you have a minute too. (I tinyurl'd the links because I want to limit search engine visibility until I'm more confident of its robustness). Also, here's the proof of ownership file.
  9. Can you run stuff from USB sticks? I've got xampplite on my USB stick and it runs fine without needing to install anything.
  10. Thanks for your reply (: The error value was 1, which I didn't realise was more meaningful that a 'yes, there was an error' until I just read that page in the manual you linked to. It would seem the uploaded files were bigger than the php.ini will allow. Causes of errors are always more obvious when someone else points them out Thanks again!
  11. Hey everyone, I'm having an intermittent problem with my upload script. It works all the time I've tested it both locally and in its live environment, and it works nearly 100% of the time in its live environment during normal usage. I've coded it so it logs any errors as and when they happen, and the log tells me that move_uploaded_file fails so returns false and that $_FILES['Filedata']['tmp_name'] is empty. I think the empty tmp_name is causing move_uploaded_file to fail, but I can't work out the cause of that problem. This is the code in question: public function upload() { if (!empty($_FILES)) { $uploadDir = 'uploads'; $subDir = uniqid(); $targetPath = $uploadDir . '/' . $subDir; $mkDir = mkdir($targetPath); $targetFile = $targetPath . '/' . $_FILES['Filedata']['name']; $tempFile = $_FILES['Filedata']['tmp_name']; $mvFile = move_uploaded_file($tempFile,$targetFile); if(!$mkDir || !$mvFile) { // Something went wrong $debug['FILES'] = $_FILES; $debug['mkdir'] = var_export($mkDir, true); $debug['mvFile'] = var_export($mvFile, true); Application_Models_Log::append($debug); echo '1'; }else { echo $targetFile; } } } The server's error_log doesn't contain anything that happened at the time of the error and as I've mentioned, I've been unable to recreate the problem. Does anybody know why the tmp_name would be empty on rare occasions? Or am I going about this in the wrong way? What else could be the problem?
  12. Hey everyone, I'm currently developing a website who's layout changes depending on the size of the device viewing it. Eg, iPhones will have each section on top of one another, landscape iPads will have 3 sections next to each other as columns and various other size devices will have slightly different set ups. I've used CSS Media queries to implement this by applying different rules depending on min/max-width and it seems to work fine when I resize my browser window to various device sizes. Am I right in thinking that this will likely work fine on the actual devices? I don't know how realistic resizing my desktop browser's window emulates the behaviour of various devices. Or, do you know of any tools that will better emulate the devices' behaviour?
  13. Or, depending on resources/budget available - you could run the code in a virtual machine that reverts back to its default state every x runs. Given the sheer number of ways you could exploit your code above, a blacklist would be very difficult to create/maintain. A whitelist would be more practical.
  14. For Facebook, I use this javascript to give permission to your website (you'll need to create a FB app). I then use their php client library (google it) to make server-calls to get the data I need. For Twitter, I use the EpiTwitter library because that handles all the oauth-ing and stuff pretty transparently.
  15. Depends on the system. 54 tables for a basic blog system? That's a hell of a lot. But 54 tables for a system that incorporates forums, blog, comments, users, profiles, photo albums etc. is not a lot.
  16. phil88

    PHP notice

    The easiest way is to just disable the display of PHP Notices by changing the error_reporting level. But just hiding the messages doesn't fix your code You could just initialise the problematic variables at the start of the code segment. Eg; $a = $b = $c = $d = $e = 0; What you initialise them to will depend on your code.
  17. 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.
  18. phil88

    PHP notice

    Different servers are set up to show different levels of errors. PHP Notices are often not shown. That particular notice occurs if you try and use a variable before it has been set. for($i = 0; $i < 5; $i++){ $var = $var + 1; } will cause PHP to issue a notice because the first time around the loop, you're trying to add 1 to $var before $var exists. To avoid this notice, you could do something like; Different servers are set up to show different levels of errors. PHP Notices are often not shown. That particular notice occurs if you try and use a variable before it has been set. $var = 0; for($i = 0; $i < 5; $i++){ $var = $var + 1; } Or, to use noXstyle's example code; for($i = 0; $i < 5; $i++){ if(isset($var)){ $var = $var + 1; }else{ $var = 1; } }
  19. Whatever the query is that will get the IDs of all the articles that are shown to the user. It'll probably be; SELECT id FROM `content`
  20. The problem, as you seem to be aware is fetching the order and the ID of the article that order applies to from the $_POST var. orderx. You could do something along the lines of; $query = mysql_query("SELECT id FROM ..."); // Get all the IDs that are to be ordered while($row = mysql_fetch_assoc($query)){ $artId = $row['id']; // Get POSTed order value $order = $_POST['order'.$artId]; $update = "UPDATE `content` SET `order` = $order WHERE `id` = $artId"; mysql_query($update); } Not secure or efficient, but I hope it's enough to get you started (:
  21. If you change; echo $thisvalue = theRefFunction($a); To; $thisvalue = theRefFunction($a); echo $thisvalue; echo $a; Echo-ing an assignment statement (what you did) doesn't make any sense. As far as preserving memory goes, your example doesn't really make sense. The &$var will prevent $a being copied when it goes into the function (thus saving memory). But it is then copied anyway when it is returned, negating the memory you saved initially. A more memory efficient function would be; function theRefFunction(&$var){ $var = $var +1; } $a = 50; theRefFunction($a); // $a is now 51 That being said, the memory benefit will be minimal. You'd need to use the function millions of times for it to make a noticeable difference. Passing by reference to save memory has bigger implications with more complicated data. PHP automatically passes objects by reference for this reason IIRC.
  22. They probably have a queue for a reason. Your best bet would be to ask them exactly what the situation is and if there's a way around it. Failing that, you could send the email using a different email provider and not use PHP's mail function. I've had a good experience with Swiftmailer before. You could probably use it to send through gmail (500 email per day limit) or some other provider.
  23. 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.
  24. 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.
  25. Maybe because he did not want google to pick up his url in a phpfreaks thread That's exactly why, thanks QuickOldCar
×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.