Skip to content

Commit

Permalink
Merge pull request #7721 from Sesquipedalian/svg_attachments
Browse files Browse the repository at this point in the history 
Improve handling of SVG image attachments and WYSIWYG attachment previews
  • Loading branch information
@Sesquipedalian
Sesquipedalian authored Sep 14, 2023
2 parents 8acca70 + a4cbaa3 commit 2148c43
Show file tree
Hide file tree
Showing 8 changed files with 309 additions and 11 deletions.
68 changes: 63 additions & 5 deletions Sources/Profile-Modify.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3536,7 +3536,8 @@ function profileSaveAvatarData(&$value)
removeAttachments(array('id_member' => $memID));

$profile_vars['avatar'] = str_replace(' ', '%20', preg_replace('~action(?:=|%3d)(?!dlattach)~i', 'action-', $_POST['userpicpersonal']));
$mime_valid = check_mime_type($profile_vars['avatar'], 'image/', true);
$mime_type = get_mime_type($profile_vars['avatar'], true);
$mime_valid = strpos($mime_type, 'image/') === 0;

if ($profile_vars['avatar'] == 'http://' || $profile_vars['avatar'] == 'http:///')
$profile_vars['avatar'] = '';
Expand All @@ -3545,6 +3546,20 @@ function profileSaveAvatarData(&$value)
return 'bad_avatar_invalid_url';
elseif (empty($mime_valid))
return 'bad_avatar';
// SVGs are special.
elseif ($mime_type === 'image/svg+xml')
{
$safe = false;

if (($tempfile = @tempnam($uploadDir, 'tmp_')) !== false && ($svg_content = @fetch_web_data($profile_vars['avatar'])) !== false && (file_put_contents($tempfile, $svg_content)) !== false)
{
$safe = checkSvgContents($tempfile);
@unlink($tempfile);
}

if (!$safe)
return 'bad_avatar';
}
// Should we check dimensions?
elseif (!empty($modSettings['avatar_max_height_external']) || !empty($modSettings['avatar_max_width_external']))
{
Expand Down Expand Up @@ -3593,11 +3608,54 @@ function profileSaveAvatarData(&$value)
$_FILES['attachment']['tmp_name'] = $new_filename;
}

$mime_valid = check_mime_type($_FILES['attachment']['tmp_name'], 'image/', true);
$mime_type = get_mime_type($_FILES['attachment']['tmp_name'], true);
$mime_valid = strpos($mime_type, 'image/') === 0;
$sizes = empty($mime_valid) ? false : @getimagesize($_FILES['attachment']['tmp_name']);

// SVGs are special.
if ($mime_type === 'image/svg+xml')
{
if ((checkSvgContents($_FILES['attachment']['tmp_name'])) === false)
{
@unlink($_FILES['attachment']['tmp_name']);
return 'bad_avatar';
}

$extension = 'svg';
$destName = 'avatar_' . $memID . '_' . time() . '.' . $extension;
list ($width, $height) = getSvgSize($_FILES['attachment']['tmp_name']);
$file_hash = '';

removeAttachments(array('id_member' => $memID));

$cur_profile['id_attach'] = $smcFunc['db_insert']('',
'{db_prefix}attachments',
array(
'id_member' => 'int', 'attachment_type' => 'int', 'filename' => 'string', 'file_hash' => 'string', 'fileext' => 'string', 'size' => 'int',
'width' => 'int', 'height' => 'int', 'mime_type' => 'string', 'id_folder' => 'int',
),
array(
$memID, 1, $destName, $file_hash, $extension, filesize($_FILES['attachment']['tmp_name']),
(int) $width, (int) $height, $mime_type, $id_folder,
),
array('id_attach'),
1
);

$cur_profile['filename'] = $destName;
$cur_profile['attachment_type'] = 1;

$destinationPath = $uploadDir . '/' . $destName;
if (!rename($_FILES['attachment']['tmp_name'], $destinationPath))
{
removeAttachments(array('id_member' => $memID));
fatal_lang_error('attach_timeout', 'critical');
}

smf_chmod($destinationPath, 0644);
}
// No size, then it's probably not a valid pic.
if ($sizes === false)
elseif ($sizes === false)
{
@unlink($_FILES['attachment']['tmp_name']);
return 'bad_avatar';
Expand Down Expand Up @@ -3664,7 +3722,7 @@ function profileSaveAvatarData(&$value)
);

$extension = isset($extensions[$sizes[2]]) ? $extensions[$sizes[2]] : 'bmp';
$mime_type = 'image/' . ($extension === 'jpg' ? 'jpeg' : ($extension === 'bmp' ? 'x-ms-bmp' : $extension));
$mime_type = str_replace('image/bmp', 'image/x-ms-bmp', $mime_type);
$destName = 'avatar_' . $memID . '_' . time() . '.' . $extension;
list ($width, $height) = getimagesize($_FILES['attachment']['tmp_name']);
$file_hash = '';
Expand Down Expand Up @@ -3698,7 +3756,7 @@ function profileSaveAvatarData(&$value)
}

// Attempt to chmod it.
smf_chmod($uploadDir . '/' . $destinationPath, 0644);
smf_chmod($destinationPath, 0644);
}
$profile_vars['avatar'] = '';

Expand Down
2 changes: 1 addition & 1 deletion Sources/ShowAttachments.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ function showAttachment()
}

// Update the download counter (unless it's a thumbnail or resuming an incomplete download).
if ($file['attachment_type'] != 3 && empty($showThumb) && $range === 0 && empty($context['skip_downloads_increment']))
if ($file['attachment_type'] != 3 && empty($showThumb) && empty($_REQUEST['preview']) && $range === 0 && empty($context['skip_downloads_increment']))
$smcFunc['db_query']('', '
UPDATE {db_prefix}attachments
SET downloads = downloads + 1
Expand Down
55 changes: 53 additions & 2 deletions Sources/Subs-Attachments.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -559,6 +559,16 @@ function attachmentChecks($attachID)
$_SESSION['temp_attachments'][$attachID]['type'] = 'image/' . $context['valid_image_types'][$size[2]];
}
}
// SVGs have their own set of security checks.
elseif ($_SESSION['temp_attachments'][$attachID]['type'] === 'image/svg+xml')
{
require_once($sourcedir . '/Subs-Graphics.php');
if (!checkSVGContents($_SESSION['temp_attachments'][$attachID]['tmp_name']))
{
$_SESSION['temp_attachments'][$attachID]['errors'][] = 'bad_attachment';
return false;
}
}

// Is there room for this sucker?
if (!empty($modSettings['attachmentDirSizeLimit']) || !empty($modSettings['attachmentDirFileLimit']))
Expand Down Expand Up @@ -687,6 +697,12 @@ function createAttachment(&$attachmentOptions)
$size = @getimagesize($attachmentOptions['tmp_name']);
list ($attachmentOptions['width'], $attachmentOptions['height']) = $size;

if (!empty($attachmentOptions['mime_type']) && $attachmentOptions['mime_type'] === 'image/svg+xml')
{
foreach (getSvgSize($attachmentOptions['tmp_name']) as $key => $value)
$attachmentOptions[$key] = $value === INF ? 0 : $value;
}

if (function_exists('exif_read_data') && ($exif_data = @exif_read_data($attachmentOptions['tmp_name'])) !== false && !empty($exif_data['Orientation']))
if (in_array($exif_data['Orientation'], [5, 6, 7, 8]))
{
Expand Down Expand Up @@ -1305,7 +1321,7 @@ function loadAttachmentContext($id_msg, $attachments)
if (!empty($attachment['id_thumb']))
$attachmentData[$i]['thumbnail'] = array(
'id' => $attachment['id_thumb'],
'href' => $scripturl . '?action=dlattach;attach=' . $attachment['id_thumb'] . ';image',
'href' => $scripturl . '?action=dlattach;attach=' . $attachment['id_thumb'] . ';image;thumb',
);
$attachmentData[$i]['thumbnail']['has_thumb'] = !empty($attachment['id_thumb']);

Expand Down Expand Up @@ -1334,6 +1350,17 @@ function loadAttachmentContext($id_msg, $attachments)

if (!$attachmentData[$i]['thumbnail']['has_thumb'])
$attachmentData[$i]['downloads']++;

// Describe undefined dimensions as "unknown".
// This can happen if an uploaded SVG is missing some key data.
foreach (array('real_width', 'real_height') as $key)
{
if (!isset($attachmentData[$i][$key]) || $attachmentData[$i][$key] === INF)
{
loadLanguage('Admin');
$attachmentData[$i][$key] = ' (' . $txt['unknown'] . ') ';
}
}
}
}

Expand Down Expand Up @@ -1362,7 +1389,7 @@ function($a, $b)
*/
function prepareAttachsByMsg($msgIDs)
{
global $context, $modSettings, $smcFunc;
global $context, $modSettings, $smcFunc, $sourcedir;

if (empty($context['loaded_attachments']))
$context['loaded_attachments'] = array();
Expand Down Expand Up @@ -1409,6 +1436,30 @@ function prepareAttachsByMsg($msgIDs)

foreach ($rows as $row)
{
// SVGs are special.
if ($row['mime_type'] === 'image/svg+xml')
{
if (empty($row['width']) || empty($row['height']))
{
require_once($sourcedir . '/Subs-Graphics.php');

$row = array_merge($row, getSvgSize(getAttachmentFilename($row['filename'], $row['id_attach'], $row['id_folder'])));
}

// SVG is its own thumbnail.
if (isset($row['id_thumb']))
{
$row['id_thumb'] = $row['id_attach'];

// For SVGs, we don't need to calculate thumbnail size precisely.
$row['thumb_width'] = min($row['width'], !empty($modSettings['attachmentThumbWidth']) ? $modSettings['attachmentThumbWidth'] : 1000);
$row['thumb_height'] = min($row['height'], !empty($modSettings['attachmentThumbHeight']) ? $modSettings['attachmentThumbHeight'] : 1000);

// Must set the thumbnail's CSS dimensions manually.
addInlineCss('img#thumb_' . $row['id_thumb'] . ':not(.original_size) {width: ' . $row['thumb_width'] . 'px; height: ' . $row['thumb_height'] . 'px;}');
}
}

if (empty($context['loaded_attachments'][$row['id_msg']]))
$context['loaded_attachments'][$row['id_msg']] = array();

Expand Down
Loading

0 comments on commit 2148c43

Please sign in to comment.