patternphpMinor
Security: Scale and cache images
Viewed 0 times
securitycacheandimagesscale
Problem
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:
Then valid requests for scaled versions include:
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:
Questions
I am wondering:
Other non-security criticisms are also welcome.
Code
The code follows:
```
* filename-#x.ext
* filename-x#.ext
* filename-#x#.ext
*
*
* 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 i
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.jpgThen valid requests for scaled versions include:
http://localhost/images/filename-200x.jpg
http://localhost/images/filename-x200.jpg
http://localhost/images/filename-200x200.jpgAll 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:
```
* filename-#x.ext
* filename-x#.ext
* filename-#x#.ext
*
*
* 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 i
Solution
This code does not do what the comment says:
Consider parsing the path using a regular expression.
For security, I've explicitly listed the allowed characters for the image filename.
// 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);Code Snippets
// Locate the position of the last hyphen in the filename.
$index = strpos( $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);Context
StackExchange Code Review Q#44025, answer score: 2
Revisions (0)
No revisions yet.