Initial mitigations for #1088 #1089

This commit is contained in:
Kijin Sung 2018-09-17 00:48:47 +09:00
parent 6d081b9fec
commit 60d390f52e
5 changed files with 79 additions and 23 deletions

View file

@ -1261,8 +1261,9 @@ class Context
$tmp_name = $val['tmp_name'];
if(!is_array($tmp_name))
{
if(!$tmp_name || !is_uploaded_file($tmp_name) || $val['size'] <= 0)
if(!UploadFileFilter::check($tmp_name, $val['name']))
{
unset($_FILES[$key]);
continue;
}
$val['name'] = escape($val['name'], false);
@ -1275,16 +1276,19 @@ class Context
$files = array();
foreach ($tmp_name as $i => $j)
{
if($val['size'][$i] > 0)
if(!UploadFileFilter::check($val['tmp_name'][$i], $val['name'][$i]))
{
$file = array();
$file['name'] = $val['name'][$i];
$file['type'] = $val['type'][$i];
$file['tmp_name'] = $val['tmp_name'][$i];
$file['error'] = $val['error'][$i];
$file['size'] = $val['size'][$i];
$files[] = $file;
$files = array();
unset($_FILES[$key]);
break;
}
$file = array();
$file['name'] = $val['name'][$i];
$file['type'] = $val['type'][$i];
$file['tmp_name'] = $val['tmp_name'][$i];
$file['error'] = $val['error'][$i];
$file['size'] = $val['size'][$i];
$files[] = $file;
}
if(count($files))
{

View file

@ -3,8 +3,60 @@
class UploadFileFilter
{
public function check($file)
/**
* Generic checker
*
* @param string $file
* @param string $filename
* @return bool
*/
public static function check($file, $filename = null)
{
// Return error if the file is not uploaded.
if (!$file || !file_exists($file) || !is_uploaded_file($file))
{
return false;
}
// Return error if the file size is zero.
if (!filesize($file))
{
return false;
}
// Get the extension.
$ext = $filename ? strtolower(substr(strrchr($filename, '.'), 1)) : '';
// Check SVG files.
if ($ext === 'svg' && !self::_checkSVG($file))
{
return false;
}
// Return true if everything is OK.
return true;
}
/**
* Check SVG file for XSS or SSRF vulnerabilities (#1088, #1089)
*
* @param string $file
* @return bool
*/
protected static function _checkSVG($file)
{
$content = file_get_contents($file);
if (preg_match('/xlink:href\s*=\s*"(?!data:)/i', $content))
{
return false;
}
if (preg_match('/<script/i', $content))
{
return false;
}
return true;
}
}

View file

@ -26,7 +26,7 @@ class fileController extends file
function procFileUpload()
{
Context::setRequestMethod('JSON');
$file_info = $_FILES['Filedata'];
$file_info = Context::get('Filedata');
// An error appears if not a normally uploaded file
if(!$file_info || !is_uploaded_file($file_info['tmp_name'])) exit();

View file

@ -128,21 +128,21 @@ class memberAdminController extends member
$signature = Context::get('signature');
$oMemberController->putSignature($args->member_srl, $signature);
$profile_image = $_FILES['profile_image'];
$profile_image = Context::get('profile_image');
if(is_uploaded_file($profile_image['tmp_name']))
{
$output = $oMemberController->insertProfileImage($args->member_srl, $profile_image['tmp_name']);
if(!$output->toBool()) return $output;
}
$image_mark = $_FILES['image_mark'];
$image_mark = Context::get('image_mark');
if(is_uploaded_file($image_mark['tmp_name']))
{
$output = $oMemberController->insertImageMark($args->member_srl, $image_mark['tmp_name']);
if(!$output->toBool()) return $output;
}
$image_name = $_FILES['image_name'];
$image_name = Context::get('image_name');
if (is_uploaded_file($image_name['tmp_name']))
{
$output = $oMemberController->insertImageName($args->member_srl, $image_name['tmp_name']);

View file

@ -692,19 +692,19 @@ class memberController extends member
if(!$output->toBool()) return $output;
// insert ProfileImage, ImageName, ImageMark
$profile_image = $_FILES['profile_image'];
$profile_image = Context::get('profile_image');
if(is_uploaded_file($profile_image['tmp_name']))
{
$this->insertProfileImage($args->member_srl, $profile_image['tmp_name']);
}
$image_mark = $_FILES['image_mark'];
$image_mark = Context::get('image_mark');
if(is_uploaded_file($image_mark['tmp_name']))
{
$this->insertImageMark($args->member_srl, $image_mark['tmp_name']);
}
$image_name = $_FILES['image_name'];
$image_name = Context::get('image_name');
if(is_uploaded_file($image_name['tmp_name']))
{
$this->insertImageName($args->member_srl, $image_name['tmp_name']);
@ -927,19 +927,19 @@ class memberController extends member
$output = $this->updateMember($args);
if(!$output->toBool()) return $output;
$profile_image = $_FILES['profile_image'];
$profile_image = Context::get('profile_image');
if(is_uploaded_file($profile_image['tmp_name']))
{
$this->insertProfileImage($args->member_srl, $profile_image['tmp_name']);
}
$image_mark = $_FILES['image_mark'];
$image_mark = Context::get('image_mark');
if(is_uploaded_file($image_mark['tmp_name']))
{
$this->insertImageMark($args->member_srl, $image_mark['tmp_name']);
}
$image_name = $_FILES['image_name'];
$image_name = Context::get('image_name');
if(is_uploaded_file($image_name['tmp_name']))
{
$this->insertImageName($args->member_srl, $image_name['tmp_name']);
@ -1056,7 +1056,7 @@ class memberController extends member
function procMemberInsertProfileImage()
{
// Check if the file is successfully uploaded
$file = $_FILES['profile_image'];
$file = Context::get('profile_image');
if(!is_uploaded_file($file['tmp_name'])) throw new Rhymix\Framework\Exception('msg_not_uploaded_profile_image');
// Ignore if member_srl is invalid or doesn't exist.
$member_srl = Context::get('member_srl');
@ -1161,7 +1161,7 @@ class memberController extends member
function procMemberInsertImageName()
{
// Check if the file is successfully uploaded
$file = $_FILES['image_name'];
$file = Context::get('image_name');
if(!is_uploaded_file($file['tmp_name'])) throw new Rhymix\Framework\Exception('msg_not_uploaded_image_name');
// Ignore if member_srl is invalid or doesn't exist.
$member_srl = Context::get('member_srl');
@ -1312,7 +1312,7 @@ class memberController extends member
function procMemberInsertImageMark()
{
// Check if the file is successfully uploaded
$file = $_FILES['image_mark'];
$file = Context::get('image_mark');
if(!is_uploaded_file($file['tmp_name'])) throw new Rhymix\Framework\Exception('msg_not_uploaded_image_mark');
// Ignore if member_srl is invalid or doesn't exist.
$member_srl = Context::get('member_srl');