4
\$\begingroup\$

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 );
  }
}
\$\endgroup\$
  • \$\begingroup\$ Comments such as Locate the position of the last hyphen in the filename. don't tell us anything new, and your code might be cleared up without those comments \$\endgroup\$ – Alex L Mar 11 '14 at 6:59
  • \$\begingroup\$ Assuming you do this for mobile, you should consider also passing quality to saveToFile, it can make a big difference in image size. \$\endgroup\$ – konijn Mar 11 '14 at 13:24
2
\$\begingroup\$

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);
\$\endgroup\$
  • \$\begingroup\$ Your test has an extra layer of quotes around the regex. \$\endgroup\$ – 200_success Mar 12 '14 at 6:20
  • 1
    \$\begingroup\$ The code works for the three examples stated in your specification. What's the failure? If you want filename-200.jpg to be treated as if it were filename-200x.jpg, you can easily tweak the regex to make the x optional (by putting a question mark after x.) \$\endgroup\$ – 200_success Mar 12 '14 at 7:35

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.