Building a Big Project, Part 5

Status
Not open for further replies.

Baljeet

Member
Jan 31, 2011
76
0
Note: this is part of a series.
The first part is here:
The second part is here:
The third part is here:
The fourth part is here:

In the last part, we had created a lovely registration menu. It sends out an email, and records entries in the database. All-in-all, it's a wonderful little tool. Unfortunately, it's a bug-fest waiting to happen, at best, and a serious vulnerability for the site, at worst.

In this section, we're going to start looking at ways to protect our site from malicious activity and general bugs. The tools we'll be using are some handy Firefox addons that can really make a difference. They are: , , , , , and . As we go through, you'll see how these can be used to create havoc with a "safe" page and review the results of malicious activity.

Let's start with a few basics. In table USERS we are populating the following values: PENNAME, PASSWORD, CONFIRMATION, EMAIL. USERS.PASSWORD will always be 128 characters, simply because it is a 128 character hash. USERS.CONFIRMATION has a similar guarantee of being 20 characters. Those two fields will never be a problem. PENNAME is limited to 30 characters, and EMAIL is limited to 100, but we don't know for sure that a user will actually cooperate with those limits. If the user doesn't, we could have problems with our INSERT statement. As a result, we really need to make sure a user won't enter data that is too long.

If you review the code in part 4, you'll see that we've used the maxlength property to provide exactly this type of protection. So, we're safe, right? Wrong! When you're programming for the web, you have to assume that the person on the other end is being malicious. That means you may not be dealing with a browser! Somebody could be telnetting into port 80 on your server and sending the appropriate strings, and completely ignoring everying. It might be a Java program! If someone is trying to be malicious, they won't honor a simple "suggestion" of only submitting 30, 50, or 100 characters. Our code will have to detect that and shut it down.

The simplest way to handle this would be to verify the length of the string, and then approve or deny the form element based on that. However, we're going to want to do a little more than just check string length. We'll be using strlen($penname) and strlen($email) in our checks.

Unfortunately, this isn't the only way a user can screw us over. SQL Injection, Cross-Site Scripting attacks (XSS), and Session vulnerabilities can all be used by a malicious "user" to cause problems. The key to all of these attacks is to trust the data a user sends. The key to preventing these attacks is to assume that every "user" is a hacker trying to compromise the site. We'll look at each vulnerability in detail below, then how to address all of them.

SQL Injection is simply an effort to add a second SQL statement on the end of the one you intended. Consider this "email" address:
Code:
  ');SELECT * FROM USERS;
If you substitute that into the following SQL from join.php:
Code:
  INSERT INTO USERS (PENNAME, PASSWORD, CONFIRMATION, EMAIL) VALUES ('$penname','$encpassword','$confirmation','$email')
you'll get:
Code:
  INSERT INTO USERS (PENNAME, PASSWORD, CONFIRMATION, EMAIL) VALUES ('$penname','$encpassword','$confirmation','  ');SELECT * FROM USERS;')
Can you see the problem? Suddenly, we have two SQL statements instead of one! Depending on what the second SQL statement is, you may have just produced a ton of sensitive data!

XSS attacks are, in their simplest form, just a script tag with malicious javascript inside. Consider the following "email" address:
Code:
  <script language="JavaScript">alert("You've been hacked!")</script>
No one should EVER accept something like this as a form input (unless you are letting someone build a site in a form)! Unfortunately, depending on how this input is handled, it could actually cause a site to put malicious code all over the place. If it were accepted as a Penname, it could be even worse. Write a story, and infect your readers!

Session vulnerabilities are another nasty vulnerability where a "user" will try to send credentials for another user's session. If successful, This could result in someone taking over a variety of users' accounts, perhaps even an administrator's!

Needless to say, join.php is not protected from any of these threats. It will need to be protected, as will the rest of the site. It's time to think about these types of vulnerabilities. Also, we need to deal with the fact that we're using "put" instead of "post" for our registration data, and the fact that we aren't using a secure form!

Security cannot be an afterthought. If it is, our site will be as vulnerable as a fresh install of Windows ME. There are three tools that can be used, explicitly, to help test our site for vulnerabilities. XSS Me, SQL Inject Me, and Access Me are plugins designed to perform basic testing on a site for vulnerabilities. These are NOT guarantees of security! SQL Inject Me, for example, reports no vulnerabilities. This is likely a result of how the test is conducted. You can play with it to get an idea. XSS Me appears to have a bug with FF 3.5.3.

In addition, you can use HackBar, Web Developer, and FireBug to do some sneaky things. Web Developer and FireBug can be used to modify the page beyond the normal limits. HackBar can help you to view URL info, post info, referrer info, and to do some sneaky stuff on the side. Being able to modify form submissions and test results is EXTREMELY important. If these plugins make it possible, that the bad guys will do it too!

So, what can we do to defend our site? To start with, we need a way to manage whether a page should be accessed over a secure connection or not. Second, we need a way to test whether a form submission is safe to display as raw text or safe to use in SQL as raw text. Third, we need a way safely encode a form submission for display or use in SQL. Fourth, we need a way to verify that form submissions are not too long. I see a need for some utility functions. Everything we develop here will be accessible for the entire site, so let's try to make it useful.

Since we want this to be accessible through the entire site, the definition to whatever we do should be found in index.php. I'm going to create two new files: init.php and close.php. init.php will contain session handling, utility functions, and establish database connections. close.php will perform cleanup. The goal is simple, to have common functionality commonly available.

Let's look at the current state of index.php:
[highlight=php]
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
<html>
<head>
<title>Myfiction.net</title>
<link rel="stylesheet" type="text/css" href="Myfiction.css" />
</head>
<body>
<?php session_start(); ?>
<?php include("header.php");?>
<?php include("menu.php");?>
<?php include("body.php");?>
<?php include("footer.php");?>
</body>
</html>
[/highlight]
Currently, it's making sure we have a session, and handing out processing to other files. We want to modify it so the session processing happens in init.php. We can do it like this:
[highlight=php]
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
<?php include("init.php");?>
<html>
<head>
<title>Myfiction.net</title>
<link rel="stylesheet" type="text/css" href="Myfiction.css" />
</head>
<body>
<?php include("header.php");?>
<?php include("menu.php");?>
<?php include("body.php");?>
<?php include("footer.php");?>
</body>
</html>
<?php include("close.php");?>
[/highlight]
Notice that session_start() is now gone. We'll stick it in init.php. The goal of init.php will be to provide all common functionality moving forward. That means, of course, that init.php will start out like this:
[highlight=php]
<?php session_start(); ?>
[/highlight]
It's not very exciting, but we all start small.

The next issue becomes, what kinds of functions should init.php provide? One possible list is:
bool IsSQLSafe(string)
bool IsHTMLSafe(string)
string MakeSQLSafe(string)
string MakeHTMLSafe(string)
bool IsValidLength(string,int)
While I like this approach, I also see a potential problem with it. It seems like the sort of code that would encourage the following:
[highlight=php]
<?php
if (IsValidLength($UserData,20))
{
if (IsSQLSafe($UserData))
{
//save data
}
else if (IsHTMLSafe($UserData))
{
//save data using MakeSQLSafe($UserData)
}
else
{
//display error message using MakeHTMLSafe($UserData)
}
}
else
{
//display generic error message
}
?>
[/highlight]
This sort of code just feels hideous to me. I think a better strategy is to make a "smart" function: string ValidateString(string,flags,int). An invalid string will have output of "". By having a list of valid flags stored in an integer (via |), we can start to get some interesting behavior. The above code would become:
[highlight=php]
<?php
if (ValidateString($UserData,$vsHTML,20))
{
//save data using ValidateString($UserData,$vsSQL)
}
else if (ValidateString($UserData,$vsNone,20))
{
//display error message using ValidateString($UserData,$vsHTML)
}
}
else
{
//display generic error message
}
?>
[/highlight]
The reason I like this approach is simple: At some point, we will probably need to add functionality. As we move forward, we will want to be able to add new types of error checking, and having an easy to remember function call to handle it. One example might be email address validation!

As an example of how we can implement this: consider this version of init.php:
[highlight=php]
<?php
session_start();

$vsNone=0;
function ValString($instr, $flags, $len=-1)
{
if (strlen($instr)>$len && $len!=-1)
$instr="";
return $instr;
}
?>
[/highlight]
I'm doing a few things here:
1) I've made $len optional. It defaults to -1, which makes it an optional parameter. Frequently we won't care about a string's length, just it's potential to scramble things.
2) I've created my first flag: $vsNone. I'm going to use that flag as the option when I want no other processing.
3) An error string is an empty string. I'll be able to use that in conditions as a "false" value.

Finishing the ValString function, and moving database connections to init.php gives us the following:
[highlight=php]
<?php
session_start();

//open database.
//This is going to store everything, so we might as well have our connection now!
if (!($sourcedb = mysql_connect('localhost:3306','root','')))
{
die('Database error: could not connect!');
}
if (!mysql_select_db('MYFICTION',$sourcedb))
{
die('Cannot use database!');
}

//set flags for ValString()
$vsNone=0;
$vsStrict=1;
$vsHTML=2;
$vsSQL=4;
$vsEmail=8;
//Validation function
//$vsStrict forces a test failure to return "", otherwise the functions will attempt to make it valid.
//
function ValString($instr, $flags, $len=-1)
{
//reintroduce flags to function scope
$vsNone=0;
$vsStrict=1;
$vsHTML=2;
$vsSQL=4;
$vsEmail=8;
//test length
if (strlen($instr)>$len && $len!=-1)
{
if ($flags & $vsStrict)
$instr="";
else
$instr=substr($instr,1,$len);
}
//test valid single email address
if ($flags & $vsEmail)
{
if(!preg_match('/^[a-zA-Z0-9._-]+@[a-zA-Z0-9-]+\.[a-zA-Z.]{2,5}$/', $instr) && ($flags & $vsStrict))
$instr="";
}
//test for invalid SQL characters
if ($flags & $vsSQL)
{
if(($flags & $vsStrict) && (pos($instr,"'") || pos($instr,"`") || pos($instr,'"')))
$instr="";
else
{
$instr=str_replace("'","''",$instr);
$instr=str_replace("`","``",$instr);
$instr=str_replace('"','""',$instr);
}
}
return $instr;
}
?>
[/highlight]

close.php just closes our connection:
[highlight=php]
<?php
//close database if still open
if ($sourcedb)
{
mysql_close($sourcedb);
}
?>
[/highlight]

Finally, our significantely more secure version of join.php:
[highlight=php]
<?php
if ($_POST["action"]=="confirm")
{
$penname = ValString($_POST["penname"],$vsStrict,20);
$password = $_POST["password"];
$passwordconf = $_POST["passwordconf"];
$email = ValString($_POST["email"],$vsStrict,100);
$emailconf = ValString($_POST["emailconf"],$vsStrict,100);
$valid = true;
if ($penname=="")
{
$valid = false;
$error = "Sorry, your User Name cannot be blank!";
}
else if ($password!=$passwordconf)
{
$valid = false;
$error = "Sorry, your passwords did not match!";
}
else if ($password=='')
{
$valid = false;
$error = "Sorry, your password cannot be blank!";
}
else if ($email!=$emailconf)
{
$valid = false;
$error = "Sorry, your emails did not match!";
}
else if ($email=="")
{
$valid = false;
$error = "Sorry, your email cannot be blank!";
}
else if (ValString($email,$vsEmail|$vsStrict)=="")
{
$valid = false;
$error = "Sorry, you have not entered a valid email address!";
}
else
{
$sql = "SELECT COUNT(*) AS MYCOUNT FROM USERS WHERE PENNAME='".ValString($penname,$vsSQL)."'";
if (!($result = mysql_query($sql, $sourcedb)))
{
echo "DB error, could not query the database";
exit;
}
$row = mysql_fetch_assoc($result);
if ($row["MYCOUNT"] != 0)
{
$valid = false;
$error = "Sorry, that pen name is already in use!";
}
}
}
else
{
$penname="";
$password="";
$passwordconf="";
$email="";
$emailconf="";
$valid = false;
$error = "";
}
if (!$valid)
{
?>
<h3>Join</h3>
<?php echo $error ?>
<form name="application" action="index.php" method="post">
<input type="hidden" name="page" value="join">
<input type="hidden" name="action" value="confirm">
<table>
<tr>
<td>
User Name:
</td>
<td>
<input name="penname" type="text" maxlength="30" value="<?php echo $penname ?>"></input>
</td>
</tr>
<tr>
<td>
Password:
</td>
<td>
<input name="password" type="text" maxlength="20" value="<?php echo $password ?>"></input>
</td>
</tr>
<tr>
<td>
Confirm Password:
</td>
<td>
<input name="passwordconf" type="text" maxlength="20" value="<?php echo $passwordconf ?>"></input>
</td>
</tr>
<tr>
<td>
Email:
</td>
<td>
<input name="email" type="text" maxlength="100" value="<?php echo $email ?>"></input>
</td>
</tr>
<tr>
<td>
Confirm Email:
</td>
<td>
<input name="emailconf" type="text" maxlength="100" value="<?php echo $emailconf ?>"></input>
</td>
</tr>
</table>
<input type="submit" value="Submit">
</form>
<?php
}
else
{
$encpassword=hash('sha512',$password);
$confirmation = "";
srand(time());
for ($i = 1; $i<=20; $i++)
$confirmation = $confirmation . (rand()%10);
$sql = "INSERT INTO USERS (PENNAME, PASSWORD, CONFIRMATION, EMAIL) VALUES ('".ValString($penname,$vsSQL)."','$encpassword','$confirmation','".ValString($email,$vsSQL)."')";
$result = mysql_query($sql, $sourcedb);
if ($result)
{
$message = 'Thank you for your interest in MyFiction.net.' . "\r\n" .
'Please confirm your registration at ' . $confirmation;
$headers = 'From: [email protected]' . "\r\n" .
'Reply-To: [email protected]' . "\r\n";
mail($email,'Thanks for joining MyFiction.net',$message,$headers)
?>
<h3>Thanks for joining!</h3>
<p>You should receive an email shortly to confirm your membership</p>
<?php
}
else
{
?>
<h3>Thanks for your interest!</h3>
<p>There was an error processing your request. Please try again later.</p>
<?php
}
}
?>
[/highlight]

There are still some problems with this code. The main one, however, is that we are NOT submitting this through a secure connection. That is easier to fix, and will be saved for later. Getting it working is slightly non-trivial, so it will wait.

Also, I'm still "accepting" "malicious" user names. That will be dealt with in displaying user names later.

All Credits goes to one who really made this...
 
Status
Not open for further replies.

Users who are viewing this thread

Top