Какие есть дыры в этих скриптах загрузки картинок?

[volodumir]

Какие есть дыры в этих скриптах загрузки картинок?

Первый скрипт:

<form enctype="multipart/form-data" action="upload.php" method="POST">
<p>
<input type="hidden" name="MAX_FILE_SIZE" value="20971520">
<input type="file" name="photo"><br>
<input type="submit" value="Загрузить">
</p>
</form>

Второй скрипт:

<?php 

$blacklist = array(".php", ".phtml", ".php3", ".php4");

foreach ($blacklist as $item) {
  if(preg_match("/$item\$/i", $_FILES['photo']['name'])) {
   echo "Хакер!!!!!!!";
   exit;
   }
  }

$type = getimagesize($_FILES['photo']['tmp_name']);

$size = $_FILES['photo']['size'];

if ($type[2] == 2&$size < 20971520)
{

$source = $_FILES['photo']['tmp_name'];
$target = 'photo' . $_FILES ['photo']['name'];
$foto = imagecreatefromjpeg ($_FILES['photo']['tmp_name']);
imagejpeg ($foto, "photo.jpg", "100");
imagedestroy ($foto);

}
elseif ($type[2] == 1&$size < 20971520)
{

$source = $_FILES['photo']['tmp_name'];
$target = 'photo' . $_FILES ['photo']['name'];
$foto = imagecreatefromgif ($_FILES['photo']['tmp_name']);
imagejpeg ($foto, "photo.jpg", "100");
imagedestroy ($foto);

}
elseif ($type[2] == 3&$size < 20971520)
{

$source = $_FILES['photo']['tmp_name'];
$target = 'photo' . $_FILES ['photo']['name'];
$foto = imagecreatefrompng ($_FILES['photo']['tmp_name']);
imagejpeg ($foto, "photo.jpg", "100");
imagedestroy ($foto);

}
else
	echo "Ваша фотография занимает больше 20 мегабайт, или вы загружаете не JPG, GIF или PNG !!!!!";

?>

Я немножко усовершенствовал свой предыдущий скрипт. Меня интересуют уезвимости моего скрипта и методы их решения.
Как заменить проверку на наличие в черном списке

$blacklist = array(".php", ".phtml", ".php3", ".php4");

foreach ($blacklist as $item) {
  if(preg_match("/$item\$/i", $_FILES['photo']['name'])) {
   echo "Хакер!!!!!!!";
   exit;
   }
  }

на белый список. То есть если файл не gif, jpg, png - то exit?

 

[korpus]

Скрипт нормальный. Уязвимостей не видно.
Для проверки типа файла следует использовать переменную $_FILES['photo']['type']

-~{}~ 30.08.10 17:30:

http://www.phpclub.ru/manrus/sec/security.html
Можете здесь кое что про безопасность прочитать

 

[Ярослав]

нужно проверять есть ли переменная isset($_FILES['photo']['tmp_name']) и проверять на ошибки $_FILES['photo']['error']
читать http://phpclub.ru/detail/article/upload

 

[Вурдалак]

Где-то я видел этот говнокодец уже. Если ты уж проверяешь неведомо зачем расширение, то проверки /\.php$/ недостаточно, т.к. файл можно сохранить под именем script.php.unknown и при запросе через Apache скрипт выполнится. Т.е. тут лишним будет знак доллара в конце паттерна.

 

[SiMM]

Вместо blacklist'а, ИМХО, разумнее whitelist.
> тут лишним будет знак доллара в конце паттерна
В котором $ в конце паттерна вполне уместен

 

[Вурдалак]

Там, кстати, будет та же дыра, если preg_quote() не использовать (имя типа «script.php.shitgif»).

 

[SiMM]

Вурдалак, дык \. в регулярке на что?
Хотя, полагаться на то, что в двойных кавычках оно не схлопнется в точку, наверно, не стоит, и лучше действительно добавить обратный слэш.

 

[Вурдалак]

SiMM, ну смотри его код, где там «\.»? Все всё поняли.

-~{}~ 31.08.10 09:26:

Автор оригинала: volodumir
Как заменить проверку на наличие в черном списке

$blacklist = array(".php", ".phtml", ".php3", ".php4");

foreach ($blacklist as $item) {
  if(preg_match("/$item\$/i", $_FILES['photo']['name'])) {
   echo "Хакер!!!!!!!";
   exit;
   }
  }

на белый список. То есть если файл не gif, jpg, png - то exit?

$whiteList = array(IMAGETYPE_GIF => '.gif', IMAGETYPE_JPEG => '.jpg', IMAGETYPE_PNG => '.png');

if( ! isset($whiteList[$imageType]) ) {
    // ...
} else {
    $filename = 'img' . $whiteList[$imageType];
}