Monday, July 12, 2010

Using the OWASP PHP ESAPI - Part 3

And here we are at the 3rd part of the OWASP ESAPI PHP tutorial series. If you haven't read the first two yet, you can find Part One here and Part Two here. This week we're going to cover encoding our output, and making our database queries safer. Let's get right into it. This week's files can be found here.

Encoding our output
We've seen the encoder in use a little bit so far - we've used it canonicalize our input. This week, we're going to use it to encode user submitted output so that we can display it safely. Our blog app is extremely vulnerable to XSS attacks right now; for example, any commentor can post malicious code. By encoding our output before we display it we can significantly cut that risk.

First step, let's examine our application and figure out where we're outputting any user input. Of our four pieces, only one of them displays user submitted data; index.php.

Last week, we said that we would not allow the user to add actual functioning HTML or JavaScript into their comments or posts, so if there is any of that, we're just going to encode it to display safely on the screen. So basically, anywhere in index.php that we're displaying something a user submitted, we're going to run it through the DefaultEncoder's encodeForHTML function. All of that is contained in our construct_content_display function, which will now look like this:

 
function construct_content_display($content_arr) {
  $output = '';
  $encoder = ESAPI::getEncoder();
  for($i=0;$i<count($content_arr);$i++) {
    $output .= "<p><b>" . $encoder->encodeForHTML($content_arr[$i]->get_title()) . "</b></p>";
    $output .= "<br><p>" . $encoder->encodeForHTML($content_arr[$i]->get_content()) . "</p>";
    $comment_arr = get_all_comments($content_arr[$i]->get_content_id());
    for($j=0;$j<count($comment_arr);$j++) {
      $output .= "<br><p>" . $encoder->encodeForHTML($comment_arr[$j]->get_comment()) . " - " . $encoder->encodeForHTML($comment_arr[$j]->get_date_created()) . "</p>";
    }
    $output .= "<br><a href=\"comment.php?content_id=" .  $content_arr[$i]->get_content_id() . "\">Comment</a>";
  }
  return $output;
}

And that's all there is to it! There are lots of other useful functions in the Encoder security control and I highly encourage you to check them all out.

Making our database queries safe
The other thing we're going to go over this week isn't completely ESAPI PHP centric. There are several tactics we could use to protect ourselves against SQL injection attacks and generally make them more error-resistant. We could use the ESAPI PHP encoder to escape our queries, but the ESAPI PHP docs for the encodeForSQL function state it best when they say:

"This method is not recommended. The use of the PreparedStatement interface is the preferred approach. However, if for some reason this is impossible, then this method is provided as a weaker alternative."

We're going to depart slightly from the ESAPI PHP for a moment to discuss using prepared statements. Prepared statements are used with the mysqli built in PHP library. They allow you to completely separate the data from the SQL making queries much safer by default. Using prepared statements is not any more difficult than using the standard PHP mysql functions, it just requires you to think a little differently. If you haven't used mysqli, you can find all the documentation here.

First thing we're going to do is modify our DB class to use mysqli now. Our new class looks like this:

 
<?php

  /**
   *
   * DB.php
   * 
   * This code is part of a tutorial on using the Open Web Application Security Project (OWASP)
   * Enterprise Security API (ESAPI) project. It is extremely insecure! Please do not use
   * this in any kind of production environment. 
   * 
   * @author jackwillk
   * @created 2010
   * 
   */

class DB {
  
  /** 
   * This class deals with mysqli singleton
   */ 

  private $host = "localhost";
  private $username = "insecureapp";
  private $password = "supersecretpw";
  private $db_name = "insecure";
  private static $instance;

  private function __construct() {
    $this->connection = new mysqli($this->host, $this->username, $this->password, $this->db_name);
  }

  static function get_instance() {
    if(!self::$instance) {
      self::$instance = new DB;
    }
    return self::$instance->connection;
  }

  }
?>

Simple! You'll notice the other thing we did was create a new user for our app which does not have root privileges. When you create the user, it's important to give the user the minimum possible privilege.

The basic steps involved in using a prepared statement are as follows:
  1. Prepare the statement
  2. Bind parameters
  3. Execute the query

Let's look at a simple example, our get_all_content function in index.php. Here's the original:

 
function get_all_content() {
  $db = DB::get_instance();
  $sql = "SELECT id FROM content order by date_created";
  $result = $db->query($sql);
  while($row=$db->fetch_assoc($result)) {
    $content_arr[] = new Content($row['id']);
  }
  return $content_arr;
}

And our new function looks like:

 
function get_all_content() {
  $db = DB::get_instance();
  $sql = $db->prepare("SELECT id, user_id, title, content, date_created FROM content ORDER BY date_created");
  $sql->execute();
  $sql->bind_result($id, $user_id, $title, $content, $date_created);
  while($sql->fetch()) {
       $post = new Content();
       $post->set_content_id($id);
       $post->set_user_id($user_id);
       $post->set_title($title);
       $post->set_content($content);
       $post->set_date_created($date_created);
       $content_arr[] = $post;
  }
  $sql->close();
  return $content_arr;
}

We prepare the statement with the prepare function and execute. After the query executes, we use the bind_result function to bind the id column to a variable called $id. The while loop using the fetch function will assign the values for each column into the variables we defined in the bind_result function, so from here we just use the setter methods to set up our object. After we've looped through our result, we clean up by closing our sql statement.

And we're going to do the same thing to our get_all_comments function:

 
function get_all_comments($content_id) {
  $db = DB::get_instance();
  $sql = $db->prepare("SELECT id, comment, content_id, date_created FROM comments WHERE content_id = ? order by date_created");
  $sql->bind_param('i', $content_id);
  $sql->execute();
  $sql->bind_result($id, $comment_body, $content_id, $date_created);
  while($sql->fetch()) {
    $comment = new Comment();
    $comment->set_comment_id($id);
    $comment->set_comment($comment_body);
    $comment->set_content_id($content_id);
    $comment->set_date_created($date_created);
    $comment_arr[] = $comment;
  }
  $sql->close();
  return $comment_arr;
}

Now we move onto our three classes. We'll start with Content.php. We use SQL queries in two of our functions, retrieve_content and write. Our new retrieve_content function looks like this:

 
  private function retrieve_content($content_id) {
    $db = DB::get_instance();
    $sql = $db->prepare("SELECT id, user_id, title, content, date_created FROM content WHERE id = ?");
    $sql->bind_param('i', $content_id);
    if(!$sql->execute()) {
      $this->error_list[] = "Could not retrieve content.";
      $sql->close;
      return false;
    }
    if(!$sql->num_rows() == 0) {
      $this->error_list[] = "Content not found.";
      $sql->close;
      return false;
    }
    $sql->bind_result($this->content_id, $this->user_id, $this->title, $this->content, $this->date_created);
    $sql->fetch();
    $sql->close();
    return true;
  }

One nice thing about the bind_result function is if we're only querying for one row, we can assign the result directly to our object properties. We're also adding some error checking into our function.

Now our write function. This is the first query where we'll actually be writing to the database with our prepared statement.

 
  function write() {
    $db = DB::get_instance();

    $sql = $db->prepare("INSERT INTO content (user_id, title, content, date_created) values (?, ?, ?, ?)");
    $sql->bind_param('isss', $this->user_id, $this->title, $this->content, date("Y-m-d"));
    if(!$sql->execute()) {
      $this->error_list[] = "Could not save post, please try again.";
      $sql->close();
      return false;
    }
    $sql->close();
    return true;
  }

Nothing too different then we've seen before. We're going to move onto the Comment class, the functions are nearly identical, retrieve_comment and write. retrieve_comment looks like:

 
  function retrieve_comment($comment_id) {
    $db = DB::get_instance();
    $sql = $db->prepare("SELECT id, comment, content_id, date_created from comments where id = ?");
    $sql->bind_param('i', $comment_id);
    if(!$sql->execute()) {
      $this->error_list[] = "Could not retrieve comment";
      $sql->close(); 
      return false;
    }
    $sql->bind_result($this->comment_id, $this->comment, $this->content_id, $this->date_created);
    $sql->fetch();
    $sql->close();
  }

And write looks like:

 
  function write() {
    $db = DB::get_instance();
    $sql = $db->prepare("INSERT INTO comments (comment, content_id, date_created) VALUES (?, ?, ?)");
    $sql->band_param("sis", $this->comment, $this->content_id, date("Y-m-d"));
    if(!$sql->execute) {
      $this->error_list[] = "Could not write comment.";
      $sql->close();
      return false;
    }
    $sql->close();
    return true;
  }

And finally, our User class. We're going to add more error handling to our class, so we're going to add the same error_list property we added to the Comment and Content classes last week. So, in the property list at the top of the class:

 
  private $error_list = null;

And our two handler functions:

 
  function clear_error_list() {
    $this->error_list = null;
  }
  
  function get_error_list() {
    return $this->error_list;
  }

Now, we improve our login function query and add better error handling:

 
  function login($username, $password) {
    $db = DB::get_instance();
    $sql = $db->prepare("SELECT id, username, password FROM user WHERE username = ? AND password = ?");
    $sql->bind_param('ss', $username, $password);
    if(!$sql->execute()) {
      $this->error_list[] = "Could not login.";
      $sql->close();
      return false;
    }
    if($sql->num_rows() == 0) {
      $this->error_list[] = "Username or password not found";;
      $sql->close();
      return false;
    }
    $sql->bind_result($this->user_id, $this->username, $this->password);
    $sql->fetch();
    $this->create_user_session();
    return true;
  }

Simple! Now, the last thing we'll do is edit our controllers to handle the new errors we're generating in our classes. So, in post.php, we'll check the return value of write() and display the error if it exists. Here's the relevant portion of post.php

 
<?php
if($_POST['submit']) {
  $content = new Content();
  $content->set_user_id($_POST['user_id']);
  $content->set_title($_POST['title']);
  $content->set_content($_POST['content']);

  $error_list = $content->get_error_list();
  
  if(!count($error_list)) {
    if($content->write()) {
      header("Location:index.php");
    } else {
      $error_list = $content->get_error_list();
    }
  } 
 }
?>


The added error handling in comment.php looks like this:

 
if($_POST['submit']) {
  //save the comment
  $comment = new Comment();
  $comment->set_comment($_POST['comment']);
  $comment->set_content_id($_POST['content_id']);
  
  $error_list = $comment->get_error_list();
  if(!count($error_list)) {
    if($comment->write()) {
      header("Location:index.php");
    } else {
      $error_list = $comment->get_error_list();
    }
  }
 }


And finally, our new login page looks like this.

 
<?php

  /**
   *
   * login.php
   * 
   * This code is part of a tutorial on using the Open Web Application Security Project (OWASP)
   * Enterprise Security API (ESAPI) project. It is extremely insecure! Please do not use
   * this in any kind of production environment 
   * 
   * @author jackwillk
   * @created 2010
   *
   */

require("lib/DB.php");
require("lib/User.php");


$user = new User();
if($user->get_user_id()) {
  echo("You are already logged in.");
  exit();
 }

if($_POST['submit']) {
  if($user->login($_POST['username'], $_POST['password'])) {
    header("Location:index.php");
  } else {
    $error_list = $user->get_error_list();
  }
 }

include("login.html");
if($error_list) {
  $content->clear_error_list();
 }
?>

And we add the error list display code to the login html page:

 
<html>
<head>
<title>Login</title>
</head>
<body>
    <?php if($error_list) { 
   for($i=0;$i<count($error_list);$i++) {
   ?>
    <font color="red"><?= $error_list[$i] ?></font><br>
    <?php } } ?>
<b>This is my awesome login page!</b>
<form name="login" method="post" action="login.php">
  username:<input type="text" name="username"><br>
  password:<input type="text" name="password"><br>
  <input type="submit" name="submit" value="submit">
</form>
</body>
</html>


And that's it for this week! This is a busy time for me - meaning it will probably be a little longer until the next post. Next time, we'll be covering logging, form security and general hardening. There's going to be one or two more posts in this series - we'll see how it goes. I would love to hear some feedback on this blog, so feel free to leave a comment, send me an email (jackwillksecurity at gmail dot com) or reach out to me on Twitter (@jackwillk)

Saturday, July 3, 2010

Using the OWASP PHP ESAPI - Part 2

After thinking about this for a week, I've decided to break this series out over a few more weeks. Rather than rush through things, I'm going to take my time and give a better overview. So, this week, I'm going to go over setting up ESAPI PHP and implementing the Validator security control. If you haven't read the first post in this series, you can find it here.

Setting up ESAPI

Setting up ESAPI PHP is very simple. Just check out the code from their Google Code repository like so:


jk@jk-laptop:/var/www/insecure_week2/lib$ svn checkout http://owasp-esapi-php.googlecode.com/svn/trunk/ owasp
A owasp/test
A owasp/test/http
A owasp/test/http/TestHttpServletRequest.php
A owasp/test/http/TestRequestDispatcher.php
...
Checked out revision 812.

Go ahead and change permissions/ownership for your own particular setup. Then just move ESAPI.xml out of your document root. We'll be modifying ESAPI.xml a little bit later, but the defaults are fine to start with. I'm actually not going to move my ESAPI.xml file (so you'll see it referenced in the ESAPI instantiation calls) just so that when we modify it, I can include it in the download link. You wouldn't want to put this into production without moving it however.

A few setup notes
We're going to use the reference objects wherever possible in ESAPI PHP (located in src/reference). They are simple implementations of the interfaces defined in src/ and for our application, we can mostly use them with minimal modifications.

We're going to use the DefaultSecurityConfiguration class, but we need to address a small bug before we can use it. So open src/reference/DefaultSecurityConfiguration.php and do a search replace. You will be replacing $this->logSpecial with $this->_logSpecial. It's incorrectly referred to in seven places.

We're also going to make a small change to the _logSpecial function - because it just prints an error to the screen, which we don't want to happen. Let's log it instead. So change the _logSpecial function in DefaultSecurityConfiguration.php to:


    private function _logSpecial($msg)
    {
      ESAPI::getAuditor('DefaultSecurityConfiguration')->warning(Auditor::SECURITY, false, $msg);
    }

I did email the ESAPI PHP mailing list about the function name issue so hopefully it will be making it into trunk in the near future.

Customizing the DefaultValidator class for our application
We're going to custom the DefaultValidator in src/reference for our application. The validator security control is used to check pieces of data against criteria we define. We can see all the documentation in the phpdocs or in the Validator.php interface in src/.

There's one thing I want to change in the DefaultValidator class. When a check fails, it throws a ValidationException that contains a message that's safe to display to the end user. The DefaultValidator catches that exception but only returns false when it catches it. Because of this, when I come back to notify the user that there was a problem with their input, I can't give them a specific reason for the failure. For example, if I check an integer input to verify that it's between 1 and 20 and the check fails, I won't know if the user entered 0, 50, or "cat". I'd like to capture that exception so that we can display the safe error message to the end user.

So we'll start by saving a copy of DefaultValidator.php as BlogValidator.php. We're going to define a new private property, $_lastError. Just put the following lines where we define the rest of the object properties.


    private $_lastError = null;

And then we need to add two methods to handle this property. getLastError to return the last error message and clearLastError to set the lastError property back to null. We're never going to set this property from anywhere but inside the BlogValidator class, but we will want to clear it after we display its value.

    /**
     * Clears the last error
     *
     * @return does not return a value
     */
    public function clearLastError() {
      $this->_lastError = null;
    }

    /**
     *  Gets the lastError property
     *
     * @return string lastError property
     */
    public function getLastError() {
      return $this->_lastError;
    }

Simple, huh? Next, we need to set this property wherever we catch an error. So in any public functions where we see

 
catch ( Exception $e)
{
 return false;
}

We're going to change it to:

 
 catch ( Exception $e )
 {
  $this->lastError = $e->getUserMessage();
  return false;
 }

And that's it! Now let's see how we would go about implementing our BlogValidator in our Content class. First, we need to include ESAPI.php and our BlogValidator.php files, so at the top of our file, we'll put:

 
require_once("owasp/src/ESAPI.php");
require_once("owasp/src/reference/BlogValidator.php");

Next we need to add some new class properties for the ESAPI controls and an error list property to hold our ValidationException error messages. Our new property declarations now look like this:

 
  private $content_id = null;
  private $user_id = null;
  private $title = null;
  private $content = null;
  private $date_created = null;
  private $esapi = null;
  private $encoder = null;
  private $validator = null;
  private $error_list = null; 

Now we'll get our ESAPI objects set up in the constructor. It's really easy. We're just using the ESAPI security control getter methods to set up the two security controls we'll be using this week. We're also going to modify our constructor just a little bit from the first week to make it easier to control the flow of our validation. We're no longer going to allow the calling method to pass all the properties into the constructor, they should use the getter/setter methods.

 
  function __construct($content_id = '') {
    $this->esapi = new ESAPI("/var/www/insecure_week2/lib/owasp/test/testresources/ESAPI.xml");
    ESAPI::setEncoder(new DefaultEncoder());
    ESAPI::setValidator(new BlogValidator());
    $this->encoder = ESAPI::getEncoder();
    $this->validator = ESAPI::getValidator();
    if($content_id) {
      $this->retrieve_content($content_id);
    }
  }

All we're doing is creating an ESAPI object and grabbing the Encoder and Validator objects. We haven't really talked about the Encoder object yet, but for now we're just going to be using it canonicalize user input before validating.

Now, we have this $error_list property, we need to handle it in basically the same we handled it in the BlogValidator class. One function to retrieve the error list, and one function to clear it.

 
  function clear_error_list() {
    $this->error_list = null;
  }

  function get_error_list() {
    return $this->error_list;
  }

Next, we'll tackle our setter methods. One thing you may have noticed is that we don't use all of these methods in our application. We could get rid of a couple of them with no effect on our app. That doesn't mean we necessarily want to get rid of them, or to ignore them when we're securing our application. At some point, we may want to use these methods as we're adding new features to our app, or if we're passing this off to another developer, who knows what they'll want to do. If we think about all of our input now, it makes our intentions clearer, and helps us to future proof the application. Also, it's less work later because the ESAPI is fresh in our minds, it's less to think about later.

Let's just go in order; we have the following functions.
  • set_content_id
  • set_user_id
  • set_title
  • set_content
  • set_date_created

There's an important step we missed the first time around, and unfortunately it's all too common for developers to miss this step. We never really thought about the kinds of values our properties should hold. We very much just slapped a bunch of crap together without much forethought into the rules that define our application. For example, we set a 140 character limit on post titles, is this reasonable? Who knows! Do we want to allow comments to put javascript in their comments? Probably not, but we didn't think about it. The only place we've really defined our object properties was in the database when we created our tables. If you think about these kind of things and define them when you're designing your application, you're a good bit of the way there.

First up, content_id. If we look at how we defined it in our database, it's a signed int, it's a primary key (so it needs to be unique), it can't be null, and MySQL will auto increment it for us if it's not defined. Now that we're considering what kind of values our object properties should contain, maybe we should start by changing our database's definition of the property.

A signed int in MySQL has a range of -2,147,483,648 to 2,147,438,648. I don't see any possible situation where we would want a negative number for an id, so maybe we should use an unsigned int. And as much as everyone's going to love reading your blog, 2 billion posts might be a bit much. Maybe we don't actually need an int where a smallint would do.

The content id is a unique id that identifies that particular piece of content, so setting it as a primary key sounds like a good idea. Finally, auto_increment is fine for a field like this because we're not particularly worried about anyone figuring out how are posts are labeled and we won't have to worry about generating unique ids.

So let's alter our database definition:

 
alter table content change column id id smallint signed auto_increment;

Now let's just enforce this in our setter method.

 
  function set_content_id($content_id) {
    $content_id = $this->canonicalize($content_id);
    if($this->validator->isValidNumber("Content ID", $content_id, 1, 65535, false)) {
      $this->content_id = $content_id;
    } else {
      $this->error_list[] = $this->validator->getLastError();
    }

  }

The only thing in our new and improved set_content_id method that you haven't seen before is the canonicalize function. This function performs canonicalization on data to ensure that it has been to its most basic form before validation (taken straight from the phpdoc). Basically, if a user is passing encoded content, this will reduce it to a form that can be validated. The encoder will throw an IntrusionException if certain problems are detected, so we need to catch that and handle it. This is why we're wrapping the canonicalize function in our Content class like so:

 
  function canonicalize($input) {
    try {
      $input = $this->encoder->canonicalize($input);
    } catch (IntrusionException $e) {
      echo($e->getUserMessage());
      exit();
    }
    return $input;
  }

We're going to do the same thing with the rest of the content properties. Our user_id is going to be the same deal as the content_id (minus the primary key and auto increment pieces). Because this user_id in content references the id in the user table, we're going to edit it there too for consistency.

 
alter table content change column user_id user_id smallint signed;
alter table user change column id id smallint signed auto_increment;

Our new set_user_id function looks like this:

 
  function set_user_id($user_id) {
    $user_id = $this->canonicalize($user_id);
    if($this->validator->isValidNumber("User ID", $user_id, 1, 10000, false)) {
      $this->user_id = $user_id;
    } else {
      $this->error_list[] = $this->validator->getLastError();
    }
       
  }

Our title property gets a little interesting. When we created our content table in the database, we defined that title as a 140 character string. We said it was OK if it was null. 140 characters seems like a reasonable limit for a post title, so we're going to keep that. But we probably don't really want it to ever be null, so we'll alter the table again.

 
alter table content change column title title varchar(140) not null;

When we're dealing with a number, it's a fairly simple matter to make sure that a user's not slipping something bad in there. With text, it's a little different. We also have to consider the output. For our simple blog, we're going to establish a few guidelines concerning the title. The post cannot insert any working HTML or javascript into their post title. A post can refer to HTML or javascript in their post titles.

This means that we want to make sure that the user input exists, doesn't exceed the maximum length and doesn't contain any non-printable characters. Beyond that, we're going to allow them to put whatever they would like and let the encoder and sanitizer objects take care of making it safe wherever we're using this property. We'll allow a user to input something potentially malicious, but we'll handle it in a safe way at our trust boundaries.

Our title setter function is going to look like this:

 
  function set_title($title) {
    $title = $this->canonicalize($title);
    if($this->validator->isValidPrintable("Post Title", $title, 140, false)) {
      $this->title = $title;
    } else {
      $this->error_list[] = $this->validator->getLastError();
    }
  }

We're essentially going to do the same exact thing for our content property. We're going to alter the database so it can't be null:

 
alter table content change column content content text not null;

and define our setter function to ensure that it only contains printable characters.

 
  function set_content($content) {
    $content = $this->canonicalize($content);
    if($this->validator->isValidPrintable("Post Content", $content, 65535, false)) {
      $this->content = $content;
    } else {
      $this->error_list[] = $this->validator->getLastError();
    }

  }

Finally, we have our date_created setter. We're just going to verify that we're using a valid date. No need to change the database here.

 
  function set_date_created($date_created) {
    $date_created = $this->canonicalize($date_created);
    if($this->validator->isValidDate("Content date created", $date_created, "Y-m-d H:i:s", false)) {
      $this->date_created = $date_created;
    } else {
      $this->error_list[] = $this->validator->getLastError();
    }

  }

We're using the validator's isValidDate function, which checks that the date passed matches the format given. The format is the same as the format used in the built in PHP date function.

So there we go, our Content class now validates its input. Here's our current Content class in full.

 
<?php
  /**
   *
   * Content.php
   * 
   * This code is part of a tutorial on using the Open Web Application Security Project (OWASP)
   * Enterprise Security API (ESAPI) project. It is extremely insecure! Please do not use
   * this in any kind of production environment 
   * 
   * @author jackwillk
   * @created 2010
   *
   */

require_once("owasp/src/ESAPI.php");
require_once("owasp/src/reference/BlogValidator.php");

class Content {
  private $content_id = null;
  private $user_id = null;
  private $title = null;
  private $content = null;
  private $date_created = null;
  private $esapi = null;
  private $encoder = null;
  private $validator = null;
  private $error_list = null;

  function __construct($content_id = '') {
    $this->esapi = new ESAPI("/var/www/insecure_week2/lib/owasp/test/testresources/ESAPI.xml");
    ESAPI::setEncoder(new DefaultEncoder());
    ESAPI::setValidator(new BlogValidator());
    $this->encoder = ESAPI::getEncoder();
    $this->validator = ESAPI::getValidator();
    if($content_id) {
      $this->retrieve_content($content_id);
    }
  }

  function get_content_id() {
    return $this->content_id;
  }

  function set_content_id($content_id) {
    $content_id = $this->canonicalize($content_id);
    if($this->validator->isValidNumber("Content ID", $content_id, 1, 65535, false)) {
      $this->content_id = $content_id;
    } else {
      $this->error_list[] = $this->validator->getLastError();
    }

  }

  function get_user_id() {
    return $this->user_id;
  }

  function set_user_id($user_id) {
    $user_id = $this->canonicalize($user_id);
    if($this->validator->isValidNumber("User ID", $user_id, 1, 10000, false)) {
      $this->user_id = $user_id;
    } else {
      $this->error_list[] = $this->validator->getLastError();
    }
       
  }

  function get_title() {
    return $this->title;
  }

  function set_title($title) {
    $title = $this->canonicalize($title);
    if($this->validator->isValidPrintable("Post Title", $title, 140, false)) {
      $this->title = $title;
    } else {
      $this->error_list[] = $this->validator->getLastError();
    }
  }

  function get_content() {
    return $this->content;
  }
  
  function set_content($content) {
    $content = $this->canonicalize($content);
    if($this->validator->isValidPrintable("Post Content", $content, 65535, false)) {
      $this->content = $content;
    } else {
      $this->error_list[] = $this->validator->getLastError();
    }

  }

  function get_date_created() {
    return $this->date_created();
  }

  function set_date_created($date_created) {
    $date_created = $this->canonicalize($date_created);
    if($this->validator->isValidDate("Content date created", $date_created, "Y-m-d H:i:s", false)) {
      $this->date_created = $date_created;
    } else {
      $this->error_list[] = $this->validator->getLastError();
    }

  }

  function clear_error_list() {
    $this->error_list = null;
  }

  function get_error_list() {
    return $this->error_list;
  }

  private function retrieve_content($content_id) {
    $db = DB::get_instance();
    $sql = "SELECT * FROM content WHERE id = " . $content_id;
    $result = $db->query($sql);
    $row = $db->fetch_assoc($result);
    $this->content_id = $row['id'];
    $this->user_id = $row['user_id'];
    $this->title = $row['title'];
    $this->content = $row['content'];
    $this->date_created = $row['date_created'];
  }

  function write() {
    $db = DB::get_instance();
    $sql = "INSERT INTO content (user_id, title, content, date_created) values ('" . $this->user_id . "', '" . $this->title . "', '" . $this->content . "', '" . date("Y-m-d") . "')";
    $result = $db->query($sql);
  }

  function canonicalize($input) {
    try {
      $input = $this->encoder->canonicalize($input);
    } catch (IntrusionException $e) {
      echo($e->getUserMessage());
      exit();
    }
    return $input;
  }

  }

?>

Let's put our Validator to good use and edit the controllers where we call these functions to deal with the errors. Lucky for us, that all takes place in one file, post.php. Because we've put so much of this into our classes, it only takes the addition of a few lines of code in our controller to implement our changes. Our new post.php looks like this:

 
<?php
  /**
   *
   * post.php
   * 
   * This code is part of a tutorial on using the Open Web Application Security Project (OWASP)
   * Enterprise Security API (ESAPI) project. It is extremely insecure! Please do not use
   * this in any kind of production environment 
   * 
   * @author jackwillk
   * @created 2010
   *
   */
require("lib/DB.php");
require("lib/User.php");
require("lib/Content.php");
require("lib/Comment.php");

$db = DB::get_instance();
$ESAPI = new ESAPI("/var/www/insecure_week2/lib/owasp/test/testresources/ESAPI.xml");

if($_POST['submit']) {
  $content = new Content();
  $content->set_user_id($_POST['user_id']);
  $content->set_title($_POST['title']);
  $content->set_content($_POST['content']);

  $error_list = $content->get_error_list();

  if(!count($error_list)) {
    $content->write();
    header("Location:index.php");
  } 
 }
$user = new User();
include("post.html");
if($error_list) {
  $content->clear_error_list();
 }
?>

And we'll edit post.html to display a list of the errors. Again, we're just adding a few easy lines of code.

 
<html>
  <head>
    <title>Post</title>
  </head>
  <body>
    <?php if($error_list) { 
   for($i=0;$i<count($error_list);$i++) {
   ?>
    <font color="red"><?= $error_list[$i] ?></font><br>
    <?php } } ?>
    Post new content<br>
    <form name="content" method="post" action="post.php">
      Title: <input type="text" name="title"><br>
      Content: <textarea name="content"></textarea><br>
      <input type="hidden" name="user_id" value="<?= $user->get_user_id() ?>">
      <input type="submit" name="submit" value="submit">
    </form>
  </body>
</html>


We'll do the same thing with our Comment class/controller/html. In our Comment class, we're going to do parameter validation, create an error list, and improve our constructor a little bit. This should all pretty much make sense if you read the Content class portion. The new Comment class looks like this:

 
<?php
  /**
   *
   * Comment.php
   * 
   * This code is part of a tutorial on using the Open Web Application Security Project (OWASP)
   * Enterprise Security API (ESAPI) project. It is extremely insecure! Please do not use
   * this in any kind of production environment 
   * 
   * @author jackwillk
   * @created 2010
   *
   */

require_once("owasp/src/ESAPI.php");
require_once("owasp/src/reference/BlogValidator.php");

class Comment {

  private $comment_id = null;
  private $comment = null;
  private $content_id = null;
  private $date_created = null;
  private $esapi = null;
  private $encoder = null;
  private $validator = null;
  private $error_list = null;

  function __construct($comment_id = '') {
    $this->esapi = new ESAPI("/var/www/insecure_week2/lib/owasp/test/testresources/ESAPI.xml");
    ESAPI::setEncoder(new DefaultEncoder());

    ESAPI::setValidator(new BlogValidator());
    $this->encoder = ESAPI::getEncoder();
    $this->validator = ESAPI::getValidator();
    if($comment_id) {
      $this->retrieve_comment($comment_id);
    }
  }

  function get_comment_id() {
    return $this->comment_id;
  }

  function set_comment_id($comment_id) {
    $comment_id = $this->canonicalize($comment_id);
    if($this->validator->isValidNumber("Comment ID", $comment_id, 1, 65535, false)) {
      $this->comment_id = $comment_id;
    } else {
      $this->error_list[] = $this->validator->getLastError();
    }
    
  }

  function get_comment() {
    return $this->comment;
  }

  function set_comment($comment) {
    $comment = $this->canonicalize($comment);
    if($this->validator->isValidPrintable("Comment", $comment, 140, false)) {
      $this->comment= $comment;
    } else {
      $this->error_list[] = $this->validator->getLastError();
    }
  }

  function get_content_id() {
    return $this->content_id;
  }

  function set_content_id($content_id) {
    $content_id = $this->canonicalize($content_id);
    if($this->validator->isValidNumber("Content ID", $content_id, 1, 65535, false)) {
      $this->content_id = $content_id;
    } else {
      $this->error_list[] = $this->validator->getLastError();
    }
  }

  function get_date_created() {
    return $this->date_created;
  }
  
  function set_date_created($date_created) {
    $date->created = $this->canonicalize($date_created);
    if($this->validator->isValidDate("Content date created", $date_created, "Y-m-d H:i:s", false)) {
      $this->date_created = $date_created;      
    } else {
      $this->error_list[] = $this->validator->getLastError();
    }

  }

  function write() {
    $db = DB::get_instance();
    $sql = "insert into comments (comment, content_id, date_created) values ('" . $this->comment . "', '" . $this->content_id . "', '" . date("Y-m-d") . "')";
    $result = $db->query($sql);
  }
  
  function canonicalize($input) {
    try {
      $input = $this->encoder->canonicalize($input);
    } catch (IntrusionException $e) {
      echo($e->getUserMessage());
      exit();
    }
    return $input;
  }

  function clear_error_list() {
    $this->error_list = null;
  }

  function get_error_list() {
    return $this->error_list;
  }

  function retrieve_comment($comment_id) {
    $db = DB::get_instance();
    $sql = "SELECT * FROM comments WHERE id = " . $comment_id;
    $result = $db->query($sql);
    $row = $db->fetch_assoc($result);
    $this->comment_id = $row['comment_id'];
    $this->comment = $row['comment'];
    $this->content_id = $row['content_id'];
    $this->date_created = $row['date_created'];
  }

}

?>


Our new comment.php controller looks like this:

 
<?php
  /**
   *
   * comment.php
   * 
   * This code is part of a tutorial on using the Open Web Application Security Project (OWASP)
   * Enterprise Security API (ESAPI) project. It is extremely insecure! Please do not use
   * this in any kind of production environment 
   * 
   * @author jackwillk
   * @created 2010
   *
   */
require("lib/DB.php");
require("lib/User.php");
require("lib/Content.php");
require("lib/Comment.php");

$db = DB::get_instance();
$ESAPI = new ESAPI("lib/owasp/test/testresources/ESAPI.xml");

if($_POST['submit']) {
  //save the comment
  $comment = new Comment();
  $comment->set_comment($_POST['comment']);
  $comment->set_content_id($_POST['content_id']);
  
  $error_list = $comment->get_error_list();
  if(!count($error_list)) {
    $comment->write();
    header("Location:index.php");
  }
 }

$content_id = $_GET['content_id'];
if(!$content_id) {
  $content_id = $_POST['content_id'];
 }
$content = new Content($content_id);
if(!$content_id) {
  echo("Missing content id");
  exit();
 }
include("comment.html");
if($error_list) {
  $content->clear_error_list();
 }

And it's html with the error list looks like:

 
<html>
  <head>
    <title>Comment</title>
  </head>
  <body>
    <?php if($error_list) { 
   for($i=0;$i<count($error_list);$i++) {
   ?>
    <font color="red"><?= $error_list[$i] ?></font><br>
    <?php } } ?>
    Comment on <?= $content->get_title() ?>
    <form name="comment" method="post" action="comment.php">
      Comment:<textarea name="comment"></textarea>
      <input type="hidden" name="content_id" value="<?= $content_id ?>">
      <input type="submit" name="submit" value="submit">
    </form>
  </body>
</html>

I think we've done quite a bit for this week. Next week, we're going to go into detail about how to handle our output and create safe database queries. You can find a link to the current project here - I didn't include the entire ESAPI PHP project, just our customized files, DefaultSecurityConfiguration.php and BlogValidator.php. They're in the main directory, you can move them into place after you check out the ESAPI PHP code.

As always, I'd love to hear feedback on this post. You can reach me at @jackwillk on Twitter, jackwillksecurity at gmail dot com, or leave a comment at the link below.