diff --git a/Sources/Profile-Modify.php b/Sources/Profile-Modify.php index 376f7f1496..9489b022cd 100644 --- a/Sources/Profile-Modify.php +++ b/Sources/Profile-Modify.php @@ -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'] = ''; @@ -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'])) { @@ -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'; @@ -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 = ''; @@ -3698,7 +3756,7 @@ function profileSaveAvatarData(&$value) } // Attempt to chmod it. - smf_chmod($uploadDir . '/' . $destinationPath, 0644); + smf_chmod($destinationPath, 0644); } $profile_vars['avatar'] = ''; diff --git a/Sources/ShowAttachments.php b/Sources/ShowAttachments.php index 1db74123d6..9644f22ba5 100644 --- a/Sources/ShowAttachments.php +++ b/Sources/ShowAttachments.php @@ -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 diff --git a/Sources/Subs-Attachments.php b/Sources/Subs-Attachments.php index 28722a7aff..8146023520 100644 --- a/Sources/Subs-Attachments.php +++ b/Sources/Subs-Attachments.php @@ -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'])) @@ -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])) { @@ -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']); @@ -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'] . ') '; + } + } } } @@ -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(); @@ -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(); diff --git a/Sources/Subs-Graphics.php b/Sources/Subs-Graphics.php index 3eb5585349..2e29595760 100644 --- a/Sources/Subs-Graphics.php +++ b/Sources/Subs-Graphics.php @@ -237,6 +237,70 @@ function checkImageContents($fileName, $extensiveCheck = false) return true; } +/** + * Searches through an SVG file to see if there's potentially harmful content. + * + * @param string $fileName The path to the file. + * @return bool Whether the image appears to be safe. + */ +function checkSvgContents($fileName) +{ + $fp = fopen($fileName, 'rb'); + if (!$fp) + fatal_lang_error('attach_timeout'); + + $patterns = array( + // No external or embedded scripts allowed. + '/<(\S*:)?script\b/i', + '/\b(\S:)?href\s*=\s*["\']\s*javascript:/i', + + // No SVG event attributes allowed, since they execute scripts. + '/\bon\w+\s*=\s*["\']/', + '/<(\S*:)?set\b[^>]*\battributeName\s*=\s*(["\'])\s*on\w+\\1/i', + + // No XML Events allowed, since they execute scripts. + '~\bhttp://www\.w3\.org/2001/xml-events\b~i', + + // No data URIs allowed, since they contain arbitrary data. + '/\b(\S*:)?href\s*=\s*["\']\s*data:/i', + + // No foreignObjects allowed, since they allow embedded HTML. + '/<(\S*:)?foreignObject\b/i', + + // No custom entities allowed, since they can be used for entity + // recursion attacks. + '/]*\bcrossorigin\s*=/', + + // No embedded PHP tags allowed. + // Harmless if the SVG is just the src of an img element, but very bad + // if the SVG is embedded inline into the HTML document. + '/<(php)?[?]|[?]>/i', + ); + + $prev_chunk = ''; + while (!feof($fp)) + { + $cur_chunk = fread($fp, 8192); + + foreach ($patterns as $pattern) + { + if (preg_match($pattern, $prev_chunk . $cur_chunk)) + { + fclose($fp); + return false; + } + } + + $prev_chunk = $cur_chunk; + } + fclose($fp); + + return true; +} + /** * Sets a global $gd2 variable needed by some functions to determine * whether the GD2 library is present. @@ -1219,4 +1283,125 @@ function showLetterImage($letter) die(); } +/** + * Gets the dimensions of an SVG image (specifically, of its viewport). + * + * See https://www.w3.org/TR/SVG11/coords.html#IntrinsicSizing + * + * @param string $filepath The path to the SVG file. + * @return array The width and height of the SVG image in pixels. + */ +function getSvgSize($filepath) +{ + preg_match('/]*>/', file_get_contents($filepath, false, null, 0, 480), $matches); + $svg = $matches[0]; + + // If the SVG has width and height attributes, use those. + // If attribute is missing, SVG spec says the default is '100%'. + // If no unit is supplied, spec says unit defaults to px. + foreach (array('width', 'height') as $dimension) + { + if (preg_match("/\b$dimension\s*=\s*([\"'])\s*([\d.]+)([\D\S]*)\s*\\1/", $svg, $matches)) + { + $$dimension = $matches[2]; + $unit = !empty($matches[3]) ? $matches[3] : 'px'; + } + else + { + $$dimension = 100; + $unit = '%'; + } + + // Resolve unit. + switch ($unit) + { + // Already pixels, so do nothing. + case 'px': + break; + + // Points. + case 'pt': + $$dimension *= 0.75; + break; + + // Picas. + case 'pc': + $$dimension *= 16; + break; + + // Inches. + case 'in': + $$dimension *= 96; + break; + + // Centimetres. + case 'cm': + $$dimension *= 37.8; + break; + + // Millimetres. + case 'mm': + $$dimension *= 3.78; + break; + + // Font height. + // Assume browser default of 1em = 1pc. + case 'em': + $$dimension *= 16; + break; + + // Font x-height. + // Assume half of font height. + case 'ex': + $$dimension *= 8; + break; + + // Font '0' character width. + // Assume a typical monospace font at 1em = 1pc. + case 'ch': + $$dimension *= 9.6; + break; + + // Percentage. + // SVG spec says to use viewBox dimensions in this case. + default: + unset($$dimension); + break; + } + } + + // Width and/or height is missing or a percentage, so try the viewBox attribute. + if ((!isset($width) || !isset($height)) && preg_match('/\bviewBox\s*=\s*(["\'])\s*[\d.]+[,\s]+[\d.]+[,\s]+([\d.]+)[,\s]+([\d.]+)\s*\\1/', $svg, $matches)) + { + $vb_width = $matches[2]; + $vb_height = $matches[3]; + + // No dimensions given, so use viewBox dimensions. + if (!isset($width) && !isset($height)) + { + $width = $vb_width; + $height = $vb_height; + } + // Width but no height, so calculate height. + elseif (isset($width)) + { + $height = $width * $vb_height / $vb_width; + } + // Height but no width, so calculate width. + elseif (isset($height)) + { + $width = $height * $vb_width / $vb_height; + } + } + + // Viewport undefined, so call it infinite. + if (!isset($width) && !isset($height)) + { + $width = INF; + $height = INF; + } + + return array('width' => round($width), 'height' => round($height)); +} + ?> \ No newline at end of file diff --git a/Sources/Subs.php b/Sources/Subs.php index 2c8d9d9f13..fc2c4bdad0 100644 --- a/Sources/Subs.php +++ b/Sources/Subs.php @@ -1769,6 +1769,9 @@ function parse_bbc($message, $smileys = true, $cache_id = '', $parse_tags = arra // Image. if (!empty($currentAttachment['is_image'])) { + // Just viewing the page shouldn't increase the download count for embedded images. + $currentAttachment['href'] .= ';preview'; + if (empty($params['{width}']) && empty($params['{height}'])) $returnContext .= ''; else diff --git a/Themes/default/scripts/jquery.sceditor.smf.js b/Themes/default/scripts/jquery.sceditor.smf.js index 6eec73779b..f02dadc563 100644 --- a/Themes/default/scripts/jquery.sceditor.smf.js +++ b/Themes/default/scripts/jquery.sceditor.smf.js @@ -728,14 +728,14 @@ sceditor.formats.bbcode.set( if (typeof attrs.height !== "undefined") attribs += ' height="' + attrs.height + '"'; - var contentUrl = smf_scripturl +'?action=dlattach;attach='+ id + ';type=preview;thumb'; + var contentUrl = smf_scripturl +'?action=dlattach;attach='+ id + ';preview;image'; contentIMG = new Image(); contentIMG.src = contentUrl; } // If not an image, show a boring ol' link if (typeof contentUrl === "undefined" || contentIMG.getAttribute('width') == 0) - return '' + content + ''; + return '' + content + ''; // Show our purdy li'l picture else return ''; diff --git a/Themes/default/scripts/script.js b/Themes/default/scripts/script.js index 8911f20abd..9397aad431 100644 --- a/Themes/default/scripts/script.js +++ b/Themes/default/scripts/script.js @@ -1538,6 +1538,7 @@ function expandThumb(thumbID) img.src = link.href; img.style.width = link.style.width; img.style.height = link.style.height; + img.classList.toggle('original_size'); // place the image attributes back link.href = tmp_src; diff --git a/Themes/default/scripts/smf_fileUpload.js b/Themes/default/scripts/smf_fileUpload.js index cd16e8b486..0a2540c7ad 100644 --- a/Themes/default/scripts/smf_fileUpload.js +++ b/Themes/default/scripts/smf_fileUpload.js @@ -173,7 +173,7 @@ function smf_fileUpload(oOptions) { // If the file is too small, it won't have a thumbnail, show the regular file. else if (typeof file.isMock !== "undefined" && typeof file.attachID !== "undefined") { - myDropzone.emit('thumbnail', file, smf_prepareScriptUrl(smf_scripturl) + 'action=dlattach;attach=' + (file.thumbID > 0 ? file.thumbID : file.attachID) + ';type=preview'); + myDropzone.emit('thumbnail', file, smf_prepareScriptUrl(smf_scripturl) + 'action=dlattach;attach=' + (file.thumbID > 0 ? file.thumbID : file.attachID) + ';preview'); } file.name = file.name.php_to8bit().php_urlencode();