Security: Scale and cache images

Question

Specification

A simple PHP script resizes images on-the-fly. The script is called by the web server's 404 handler to return a scaled version of the original. For example, if the original image is at:

http://localhost/images/filename.jpg

Then valid requests for scaled versions include:

http://localhost/images/filename-200x.jpg
http://localhost/images/filename-x200.jpg
http://localhost/images/filename-200x200.jpg

All requests maintain the aspect ratio. The first request scales to the image to 200px wide; the second scales the image to 200px high; the third request ensures neither the width nor height exceeds 200x200px.

This is similar to how Google Picasa works.

Assumptions

The code is called with the following known constraints:

• The file names are hashed.
• The images are valid.
• The images are in a format recognized by the WideImage library.
• Each image typically resides in its own folder, nested a few directories deep.

Questions

I am wondering:

• What security holes are present?
• How would you address them?
• What optimizations are possible (e.g., use a faster library)?

Other non-security criticisms are also welcome.

Code

The code follows:

<?php
/**
 * This file handles dynamically caching and resizing thumbnails. This
 * file is called when the web server encouters a 404 error on a thumbnail
 * image. The image is created and returned, which will prevent subsequent
 * 404s on that image. The file names must be formatted as:
 *
 * <pre>
 *   filename-#x.ext
 *   filename-x#.ext
 *   filename-#x#.ext
 * </pre>
 *
 * The examples specify: (1) width; (2) height; (3) width and height. The
 * aspect ratio is maintained.
 *
 * For security purposes, only a specific set of dimensions are allowed.
 */

include 'WideImage/WideImage.php';

// Limit the number of possible cached images (i.e., do not allow arbitrary
// dimensions to create cached files).
$ACCEPT_DIMENSIONS = array( 200, 1024, 2048, 4096 );

// The "images" suffix comes from the URL.
$IMAGES_DIRECTORY = "/home/data";

$DIVIDER = "x";

/**
 * Given an integer value, this will find its nearest value within an
 * array of values.
 *
 * @param $needle The value whose nearest value is sought.
 * @param $haystack The valid values.
 *
 * @return The value in $haystack that is nearest to $needle.
 * @see http://stackoverflow.com/a/5464961/59087
 */
function nearest( $needle, $haystack ) {
  $near = null;

  foreach( $haystack as $hay ) {
    if( $near == null || abs( $needle - $near ) > abs( $hay - $needle ) ) {
      $near = $hay;
    }
  }

  return $near;
}

/**
 * Returns the minimum dimension of $dim_a or $dim_b such that the resulting
 * value is nearest to the acceptable array of dimensions.
 *
 * @param $dim_a The first dimension for comparison.
 * @param $dim_b The second dimension for comparison.
 *
 * @return The smaller of dim_a and dim_b evaluated to the nearest value
 * in the ACCEPT_DIMENSIONS array.
 */
function minimum( $dim_a, $dim_b ) {
  global $ACCEPT_DIMENSIONS;
  return nearest( min( $dim_a, $dim_b ), $ACCEPT_DIMENSIONS );
}

// Separate the query string into its path parts (filename, directory, etc.).
$path = pathinfo( $_SERVER["QUERY_STRING"] );

// Get the filename without the extension.
$filename = $path["filename"];

// Get the path to the image (appended to $IMAGES_DIRECTORY).
$dirname = $path["dirname"];

// Get the image extension, which will be used when saving the scaled file.
$extension = $path["extension"];

// Locate the position of the last hyphen in the filename.
$index = strpos( $filename, "-" );

// When one hyphen is present, it indicates dimensions might be specified...
if( $index > 1 && substr_count( $filename, "-", $index ) === 1  ) {
  // Grab the filename without the specified dimensions.
  $original_filename = substr( $filename, 0, $index );
  $dimensions = substr( $filename, $index + 1 );

  // If the x isn't present, ensure it will be parsed and used in the
  // output filename.
  if( strpos( $dimensions, $DIVIDER ) === false ) {
    $dimensions .= $DIVIDER;
    $filename .= $DIVIDER;
  }

  // Split a maximum of 2 values: the width and height. The "x" is always
  // present.
  list( $width, $height ) = explode( $DIVIDER, $dimensions, 2 );

  // Ensure no funny stuff has happened with the numbers.
  $width = intval( $width );
  $height = intval( $height );

  // Determine the full path to the original file.
  $source = "$IMAGES_DIRECTORY$dirname/$original_filename.$extension";

  // Open the original image to scale to the given dimensions.
  $image = WideImage::load( $source );

  // Scaling cannot exceed original dimensions; the resize API requires
  // "null" values to maintain aspect ratios.
  // @see http://wideimage.sourceforge.net/
  $width = empty( $width ) ? null : minimum( $width, $image->getWidth() );
  $height = empty( $height ) ? null : minimum( $height, $image->getHeight() );

  // If the current URI differs from the sanitized URI then updates are needed.
  $requested = "$IMAGES_DIRECTORY$_SERVER[REQUEST_URI]";
  $sanitized = "$IMAGES_DIRECTORY$dirname/$original_filename-$width$DIVIDER$height.$extension";

  // Set to false when resizing and saving is not required.
  $update = true;

  // If the requested filename and the filename with acceptable dimensions
  // differ, then create the file with acceptable dimensions, provided it
  // does not already exist.
  if( $requested !== $sanitized ) {
    if( file_exists( $sanitized ) ) {
      // Load up the existing image for sending to the browser.
      $image = WideImage::load( $sanitized );

      // No need to resize or save.
      $update = false;
    }
  }

  if( $update ) {
    $image = $image->resize( $width, $height );
  }

  $image->output( $extension );

  if( $update ) {
    // Save the file (so that the web server can find it next time). This
    // could fail silently, so prefer sending the image before saving it.
    $image->saveToFile( $sanitized );
  }
}

Answer 1

This code does not do what the comment says:

// Locate the position of the last hyphen in the filename.
$index = strpos( $filename, "-" );

---

Consider parsing the path using a regular expression.

For security, I've explicitly listed the allowed characters for the image filename.

# You probably want to analyze PATH_INFO rather than QUERY_STRING
$path = pathinfo($_SERVER['PATH_INFO']);
if (!preg_match('!^([A-Za-z0-9_]+)-(\d+)?x(\d+)?$!', $path['filename'], $matches)) {
    header('HTTP/1.0 404 Not Found');
    return;
}
$orig_image = sprintf('%s%s/%s.%s',
                      $IMAGES_DIRECTORY, $path['dirname'],
                      $matches[1], $path['extension']);
$width  = $matches[2];
$height = $matches[3];
if (!file_exists($orig_image)) {
    header('HTTP/1.0 404 Not Found');
    return;
}

# That's it for the URL parsing code!  Let's get on with the real work!
serve_image($orig_image, $width, $height);