Page 1 of 1

Security holes, poor syntax, convoluted reasoning... help!

Posted: Thu Jul 12, 2007 8:36 am
by Chalks
Hello, hello! I am brand spankin' new to php (and this forum!)... I started learning it two days ago. However, I'm fluent in html, css, java, javascript, and c++, so it was easy to learn a large part of it. I have a ton of questions about the following bit of code. I wasn't sure whether to post it here, or in php - code, or in security, so if it's in the wrong place, sorry!

what the script does: This is supposed to password protect a page. Once the user gets the correct username/password they can edit the contents of a file named index.dat. When they finish editing the file, the website (index.php) reads the content of the .dat file, and puts it wherever I want it to. This is so that my customers, who know almost no html, can safely (and easily) edit the content of their website.

Code: Select all

<?php
session_start();

$user="theuser";
$pass="1234567";
$loggedIn=false;

print "<html>";
print "<body>";

  function write()
  {
//    if (get_magic_quotes_gpc())
//    {
//      $_POST = array_map('stripslashes', $_POST);
//    }

    $text=$_POST["text"];
    $file=fopen("index.dat","w+") or exit("data corrupted, please contact webmaster!");
    echo fwrite($file, $text);
    fclose($file);
    header("location: admin.php");
  }

  function logout()
  {
    $_SESSION = array();
    if (isset($_COOKIE[session_name()]))
    {
      setcookie(session_name(), '', time()-42000, '/');
    }
    session_destroy();
    header("location: admin.php");
  }


if((isset($_SESSION['user']) && isset($_SESSION['pass'])) || (isset($_POST['pass']) && $_POST['pass']==$pass && isset($_POST['user']) && $_POST['user']==$user))
{
  $_SESSION['user']=$_POST['user'];
  $_SESSION['pass']=$_POST['pass'];

  $loggedIn=true;

  print "<form action=\"$PHP_SELF\" method=\"post\">";
  print "<textarea rows=\"20\" cols=\"70\" name=\"text\">";
  $file=fopen("index.dat","r") or exit("data corrupted, please contact webmaster!");
  while(!feof($file))
  {
    print fgets($file);
  }
  fclose($file);

  print "</textarea>";
  print "<br /><input type=\"submit\" value=\"Submit\" />";
  print "</form>";

  print "<br /><br /><a href=\"admin.php?logout\">Logout</a>";

  if(isset($_GET['logout']))
  {
    logout();
  }

  if(isset($_POST['text']))
  {
    write();
  }
}
else
{
  
  print "<form action=$PHP_SELF method=\"post\"><p align=\"center\">Please enter your username and password for access<br />";
  if(isset($_POST['pass']) || $pass=="" || isset($_POST['user']) || $user=="")
  {
    print "<font color=\"#FF0000\">Invalid username or password!</font>";
  }
  print "<br /><input name=\"user\" size=\"25\" maxlength=\"10\">";
  print "<br /><input name=\"pass\" type=\"password\" size=\"25\" maxlength=\"10\">";
  print "<br /><input value=\"Login\" type=\"submit\"></p></form>";
}

print "</body></html>";

?>
questions:
1. What security holes exist, and how do I fix them? I know having my passwords in plain text is NOT a good idea. How (as in, what code do I use) do I use md5 to encrypt them, or hashs, or whatever is considered "best"? If it involves using a mySQL database, that's cool... I'm reading the w3schools.com docs on mysql right now. Also, the php file is currently in a public directory. If it should be below root or in a protected directory, how do I include it in a different file? Just use ../ enough times to get past the root? or use the actual path?

2. The script is designed to modify a file that my user's website then reads. However, the file that I'm modifying is in the same folder on my server, and the only way I could modify it was if I set the permissions on index.dat to 646. This is probably incredibly unsafe. Should I put my index.dat file into a password protected directory on my server and then access it from the script? If so, how? Or is there a better way to do what I'm attempting without a intermediate file (index.dat)?

3. How do I make sure that the string passed to my .dat file doesn't include particular characters (such as <>"';: etc.)? Magic quotes of course does a large part of this, but magic quotes also screws up words like "it's". Basically... how can I say "if ($string includes(<, >, ", ')) { echo "Stop that!"; }"? EDIT: I think I found the answer up in the regex forum.

4. Am I destroying the session correctly? How do I make it time out after X minutes? When I have a session running, how can I have a different php page (like... my index.php instead of just the admin.php) know that the user is logged in? Is it as simple as saying "if(isset($_SESSION)){ do stuff }"?

5. I'm sure this is some of the messiest code ever. I've got html mixed in with php and it was getting confusing. What advice do you guys have for cleaning it up and making it easier to read.

6. If I have an if statement like so:

Code: Select all

<?php if(stuff) {?>
does the if statement carry over to the next php tag, or do I have to close the statement before the close tag?

7. Is there a way to code "when I click this button/link, perform this php function"? I kind of jerry-rigged a way to do that but... it's not very efficient.

8. Finally... I was getting a "can not insert into header" error from my logout link. I suppressed the errors in the .htaccess file in my root directory and just made my logout link reload the page. That appeared to fix the problem but, I'm nervous about it.



Whew. That was a lot. Before I get a lot of "google it!" responses, I _have_ googled (and google codesearched... man I love that thing) all of these questions and am in the process of learning the answers (this post is part of the process). However I can only read so much theory before I have to just do it, otherwise I don't learn anything. So, please be gentle! :D

THANKS!



edit: Oh... if you have a favorite "learn php" or "here are common questions and help" website, I'd love to check it out. :)

Posted: Thu Jul 12, 2007 4:15 pm
by Ambush Commander
What security holes exist, and how do I fix them?
That's difficult to say. We'll find out.
How (as in, what code do I use) do I use md5 to encrypt them, or hashs, or whatever is considered "best"? If it involves using a mySQL database, that's cool... I'm reading the w3schools.com docs on mysql right now.
In this case, it's reasonable. You could roll a MySQL based authentication system but that seems like overkill for now.

For extra protection, don't include the password directly in the PHP file: instead, take the sha1 hash (md5 has security problems) of your desired password, and put that in the PHP file. Then, compare them as such: $pass == sha1($_POST['pass'])
Also, the php file is currently in a public directory. If it should be below root or in a protected directory, how do I include it in a different file? Just use ../ enough times to get past the root? or use the actual path?
Because PHP is interpreted, the contents of the PHP file should never be made public. Storing the configuration in another file below the web root can lend security in case the PHP interpreter croaks, but I know many applications don't bother.
The script is designed to modify a file that my user's website then reads. However, the file that I'm modifying is in the same folder on my server, and the only way I could modify it was if I set the permissions on index.dat to 646. This is probably incredibly unsafe. Should I put my index.dat file into a password protected directory on my server and then access it from the script? If so, how? Or is there a better way to do what I'm attempting without a intermediate file (index.dat)?
What's probably happening is that the webserver runs as a different user than you; if that's the case, using 666 is the only way to do this. The file will not be vulnerable from outside users; only users on the same system who know the location of the file (assuming you've made your home directory non-readable to the world) can edit it.

You can get around the file permission problems by using a database.
How do I make sure that the string passed to my .dat file doesn't include particular characters (such as <>"';: etc.)? Magic quotes of course does a large part of this, but magic quotes also screws up words like "it's". Basically... how can I say "if ($string includes(<, >, ", ')) { echo "Stop that!"; }"? EDIT: I think I found the answer up in the regex forum.
You shouldn't be using magic quotes, in fact, you should be un-doing them, and then raw characters to the data file. (use mysql_real_escape_string if you're entering into a database).

Also, remember that headers need to be sent before any output (i.e. print()ed stuff)

Posted: Thu Jul 12, 2007 5:19 pm
by Christopher
The general rules are:

1. Validate and/or filter all input

2. Escape all output using the appropriate function

Posted: Fri Jul 13, 2007 3:00 pm
by Chalks
Ambush Commander wrote:Also, remember that headers need to be sent before any output (i.e. print()ed stuff)
I kind of figured that, but the reason I included them where I did was because when the code reached that point, it would reload the page and resend all data. I wasn't sure how else to update my .dat file. I think that problem will be easily solved when I switch to a mysql database though.
arborint wrote:2. Escape all output using the appropriate function
I hate to ask, because I'm sure I should know the answer but... what do you mean by "escape output"?


Edit: oh, as an aside... is there a huge difference between "print" and "echo" commands?

Posted: Fri Jul 13, 2007 5:43 pm
by lightningstrike
Escaping output can be done with functions such as mysql_real_escape_string, htmlentities($variable,ENT_QUOTES).
4. Am I destroying the session correctly? How do I make it time out after X minutes? When I have a session running, how can I have a different php page (like... my index.php instead of just the admin.php) know that the user is logged in? Is it as simple as saying "if(isset($_SESSION)){ do stuff }"?
Yes, the session carries over to multiple pages as the Session ID is stored in a cookie. However it is limited to the domain it is issued to. Meaning viewing the website through its ip not the regular domain will create seperate sessions. If you want the session to time out after X minutes simply edit your .htaccess file and adding in php_flag session.gc_maxlifetime = X (seconds).
6. If I have an if statement like so:

Code: Select all

<?php if(stuff) {?>
does the if statement carry over to the next php tag, or do I have to close the statement before the close tag?
Yes, the if statement will carry over to the next php tag.
8. Finally... I was getting a "can not insert into header" error from my logout link. I suppressed the errors in the .htaccess file in my root directory and just made my logout link reload the page. That appeared to fix the problem but, I'm nervous about it.
This might be due to you sending http headers to the browser by printing html before calling a function such as session_start() or session_destroy() or header(). Not certain without the full error message.
Edit: oh, as an aside... is there a huge difference between "print" and "echo" commands?
No there isn't any major difference between the two constructs. The only difference is echo is marginally faster as it does not return a value unlike print