A little hole in script [Archive]

View Full Version : A little hole in script

Martinss

01-18-2008, 07:26 PM

Hello there everybody!

At first - sorry, for my bad english.

I wanted to ask a little help in PHP.
At first - i got this image upload site img.lejup.lv
And theres a little problem.

Normaly, when image displays with viewer, it displays like that:

img.lejup.lv/viewerisz.php?id=59.jpg

But someone found, that there could be added something more in browser bar, to get this:

img.lejup.lv/viewerisz.php?id=%27%3E%3Ch1%3EU%20FAIL

Can someone give me any solution?

viewerisz.php code is right below:

<?php
include "konfiguracija.php";
echo "<center>";
echo "<br>";
if ($_GET['id'])
{
$id = $_GET['id'];
}
else
{
die ("Nepareizs attela ID");
}

echo "<body bgcolor='#FFFFFF'>";
echo "<table border='0' bgcolor='FFFFFF'>";
echo "<tr><td>";
echo "<img src='./$path" . $id . "'>";
echo '</td><a href="javascript:history.go(-1);" title="uz iepriekšējo lapu">Atpakaļ</a> </tr>';
echo "</div>";

?>

All simple upload holes i repaired, but with this simple one, can't get right.

Harzem

01-18-2008, 07:48 PM

After this line:

$id = $_GET['id'];

Put this:

$id = htmlspecialchars($id);

Also, replace this line:

if ($_GET['id'])

with this:

if ($_GET['id'] && strpos($_GET['id'],"..") === false && is_file("./$path".$_GET['id']))

should provide some sort of security.

taec

01-18-2008, 07:51 PM

Hi.

Personally, I would change your script a little bit. Give every single image you upload a unique key. Easiest way to do that would be to create an MD5 hash of the filename plus microtime() plus a simple random number.

Store that key somewhere (database preferable) along with the filename.

Use that key for referencing the file.

A simpler way for the moment, might be to restrict what characters are used in files that are uploaded. Keep it simple, like A-Z, a-z, 0-9 underscore, dash and period characters. Then match all files against a regular expression which checks them for these.

The first way is better for a number of reasons however.

Hope that helps.

Martinss

01-19-2008, 07:47 AM

Harzem, thank you - it works. :)

Taec - my skill in mysql and in PHP is very poor.

Steve_Arm

01-19-2008, 09:22 AM

If the ID is always a number you can use this:

<?php
$id = $_GET['id'];
if (!preg_match('/^[0-9]+\.(jpg|png|gif)$/i', $id))
{
die ("Nepareizs attela ID");
}
include "konfiguracija.php";
$pathtofile = $path.$id;
if (!is_file($pathtofile))
{
die ("Nepareizs attela ID");
}
$str = '<center>';
$str .= '<br />';
$str .= '<body bgcolor="#FFFFFF">';
$str .= '<table border="0" bgcolor="#FFFFFF">';
$str .= '<tr><td>';
$str .= '<img src="'.$pathtofile.'">';
$str .= '</td><a href="javascript:history.go(-1);" title="uz iepriekšējo lapu">Atpakaļ</a></tr>';
$str .= '</div>';
echo $str;
unset($pathtofile, $str);
?>

Martinss

01-19-2008, 04:18 PM

Already using Harzem solution, but thanks for help Steve_Arm. :)