From 7070a1448066e14bc30c360accf7b0a686628eab Mon Sep 17 00:00:00 2001 From: Miguel Dantas Date: Thu, 11 Jul 2019 23:49:16 +0100 Subject: [PATCH] [CORE][StoreRemoteMedia] Fixed bug where sometimes images were written outside the site root --- actions/attachment.php | 3 +- classes/File.php | 8 ++--- classes/File_thumbnail.php | 8 ++--- lib/httpclient.php | 15 ++++++++++ lib/imagefile.php | 29 +++++++------------ lib/mediafile.php | 7 +++-- .../StoreRemoteMediaPlugin.php | 22 +++++++++----- 7 files changed, 53 insertions(+), 39 deletions(-) diff --git a/actions/attachment.php b/actions/attachment.php index f261cc30bd..993f94b007 100644 --- a/actions/attachment.php +++ b/actions/attachment.php @@ -208,7 +208,8 @@ class AttachmentAction extends ManagedAction */ static function sendFile(string $filepath, $filesize) { if (is_string(common_config('site', 'x-static-delivery'))) { - $relative_path = end(explode(INSTALLDIR, $filepath)); + $tmp = explode(INSTALLDIR, $filepath); + $relative_path = end($tmp); common_debug("Using Static Delivery with header: '" . common_config('site', 'x-static-delivery') . ": {$relative_path}'"); header(common_config('site', 'x-static-delivery') . ": {$relative_path}"); diff --git a/classes/File.php b/classes/File.php index 564bdb0055..95127c6aee 100644 --- a/classes/File.php +++ b/classes/File.php @@ -617,7 +617,7 @@ class File extends Managed_DataObject } } else { try { - return File_thumbnail::byFile($this)->getPath(); + return File_thumbnail::byFile($this, true)->getPath(); } catch (NoResultException $e) { // File not stored locally throw new FileNotStoredLocallyException($this); @@ -637,10 +637,10 @@ class File extends Managed_DataObject { if (!empty($thumbnail)) { $filepath = $thumbnail->getPath(); - } elseif (empty($this->filename)) { - $filepath = File_thumbnail::byFile($this)->getPath(); - } else { + } elseif (!empty($this->filename)) { return $this->mimetype; + } else { + $filepath = File_thumbnail::byFile($this, true)->getPath(); } $info = @getimagesize($filepath); diff --git a/classes/File_thumbnail.php b/classes/File_thumbnail.php index 5e751c3c4d..eb6295501a 100644 --- a/classes/File_thumbnail.php +++ b/classes/File_thumbnail.php @@ -244,11 +244,9 @@ class File_thumbnail extends Managed_DataObject */ public function getHtmlAttrs(array $orig=array(), $overwrite=true) { - $attrs = [ - 'height' => $this->getHeight(), - 'width' => $this->getWidth(), - 'src' => $this->getUrl(), - ]; + $attrs = [ 'height' => $this->getHeight(), + 'width' => $this->getWidth(), + 'src' => $this->getUrl() ]; return $overwrite ? array_merge($orig, $attrs) : array_merge($attrs, $orig); } diff --git a/lib/httpclient.php b/lib/httpclient.php index 819a5e6e07..184b2c2e13 100644 --- a/lib/httpclient.php +++ b/lib/httpclient.php @@ -402,4 +402,19 @@ class HTTPClient extends HTTP_Request2 } while ($maxRedirs); return new GNUsocial_HTTPResponse($response, $this->getUrl(), $redirs); } + + public static function get_filename(string $url, array $headers = null) : string { + if ($headers === null) { + $head = (new HTTPClient())->head($url); + $headers = $head->getHeader(); + $headers = array_change_key_case($headers, CASE_LOWER); + } + if (array_key_exists('content-disposition', $headers) && + preg_match('/^.+; filename="(.+?)"$/', $headers['content-disposition'], $matches) === 1) { + return $matches[1]; + } else { + common_log(LOG_INFO, "Couldn't determine filename for url: {$url}"); + return _('Untitled attachment'); + } + } } diff --git a/lib/imagefile.php b/lib/imagefile.php index c6465c52b1..b921a2483c 100644 --- a/lib/imagefile.php +++ b/lib/imagefile.php @@ -125,12 +125,10 @@ class ImageFile extends MediaFile } // And we'll only consider it an image if it has such a media type - switch ($media) { - case 'image': - $imgPath = $file->getPath(); - break; - default: + if($media !== 'image') { throw new UnsupportedMediaException(_m('Unsupported media format.'), $file->getPath()); + } else if (!empty($file->filename)) { + $imgPath = $file->getPath(); } } @@ -376,7 +374,11 @@ class ImageFile extends MediaFile $type = $this->preferredType(); $ext = image_type_to_extension($type, true); - $outpath = preg_replace("/\.[^\.]+$/", $ext, $outpath); + // Decoding returns null if the file is in the old format + $filename = MediaFile::decodeFilename(basename($outpath)); + // Encoding null makes the file use 'untitled', and also replaces the extension + $outfilename = MediaFile::encodeFilename($filename, $this->filehash, $ext); + $outpath = dirname($outpath) . DIRECTORY_SEPARATOR . $outfilename; switch ($type) { case IMAGETYPE_GIF: @@ -573,9 +575,8 @@ class ImageFile extends MediaFile return $thumb; } - $filename = basename($this->filepath); $extension = File::guessMimeExtension($this->mimetype); - $outname = "thumb-{$this->fileRecord->getID()}-{$width}x{$height}-{$filename}"; + $outname = "thumb-{$this->fileRecord->getID()}-{$width}x{$height}-{$this->filename}"; $outpath = File_thumbnail::path($outname); // The boundary box for our resizing @@ -601,16 +602,8 @@ class ImageFile extends MediaFile )); // Perform resize and store into file - $this->resizeTo($outpath, $box); - - try { - // Avoid deleting the original - if (!in_array($this->getPath(), [File::path($this->filename), File_thumbnail::path($this->filename)])) { - $this->unlink(); - } - } catch (FileNotFoundException $e) { - // $this->getPath() says the file doesn't exist anyway, so no point in trying to delete it! - } + $outpath = $this->resizeTo($outpath, $box); + $outname = basename($outpath); return File_thumbnail::saveThumbnail( $this->fileRecord->getID(), diff --git a/lib/mediafile.php b/lib/mediafile.php index 0efdbada76..5086add583 100644 --- a/lib/mediafile.php +++ b/lib/mediafile.php @@ -241,7 +241,7 @@ class MediaFile * having an extension in the file, removing trust in extensions, while keeping the original name * @throws ClientException */ - public static function encodeFilename(string $original_name, string $filehash, string $ext = null) : string + public static function encodeFilename($original_name, string $filehash, $ext = null) : string { if (empty($original_name)) { $original_name = _('Untitled attachment'); @@ -274,7 +274,8 @@ class MediaFile */ public static function decodeFilename(string $encoded_filename) { - $ret = preg_match('/^([^-]+?)-[^-]+$/', $encoded_filename, $matches); + // The x is because it is using in thumbnails + $ret = preg_match('/^([^-x]+?)-[^-]+$/', $encoded_filename, $matches); if ($ret === false) { return false; } elseif ($ret === 0) { @@ -650,7 +651,7 @@ class MediaFile } elseif ($filename === null) { // The old file name format was "{hash}.{ext}" so we didn't have a name // This extracts the extension - $ret = preg_match('/^.+?\.(.+)$/', $file->filename, $matches); + $ret = preg_match('/^.+?\.+?(.+)$/', $file->filename, $matches); if ($ret !== 1) { common_log(LOG_ERR, $log_error_msg); return _('Untitled attachment'); diff --git a/plugins/StoreRemoteMedia/StoreRemoteMediaPlugin.php b/plugins/StoreRemoteMedia/StoreRemoteMediaPlugin.php index a722f9535a..582fc5f4e0 100644 --- a/plugins/StoreRemoteMedia/StoreRemoteMediaPlugin.php +++ b/plugins/StoreRemoteMedia/StoreRemoteMediaPlugin.php @@ -75,8 +75,15 @@ class StoreRemoteMediaPlugin extends Plugin 'unnecessarily downloading too large files. URL: %s', $file->getID(), $remoteUrl)); + $url = $remoteUrl; $head = $http->head($remoteUrl); $remoteUrl = $head->getEffectiveUrl(); // to avoid going through redirects again + + if (empty($remoteUrl)) { + common_log(LOG_ERR, "URL after redirects is somehow empty, for URL {$url}"); + return true; + } + if (!$this->checkBlackList($remoteUrl)) { common_log(LOG_WARN, sprintf('%s: Non-blacklisted URL %s redirected to blacklisted URL %s', __CLASS__, $file->getUrl(), $remoteUrl)); @@ -84,6 +91,7 @@ class StoreRemoteMediaPlugin extends Plugin } $headers = $head->getHeader(); + $headers = array_change_key_case($headers, CASE_LOWER); $filesize = isset($headers['content-length']) ?: $file->getSize(); if (empty($filesize)) { @@ -133,18 +141,16 @@ class StoreRemoteMediaPlugin extends Plugin //FIXME: Add some code so we don't have to store duplicate File rows for same hash files. } catch (NoResultException $e) { - if (preg_match('/^.+; filename="(.+?)"$/', $headers['content-disposition'], $matches) === 1) { - $filename = MediaFile::encodeFilename($matches[1], $filehash); - } else { - common_log(LOG_ERR, "Couldn't determine filename for url: {$remoteUrl}"); - // throw new ServerError(_("Couldn't determine filename for url: {$remoteUrl}")); - } + $original_name = HTTPClient::get_filename($remoteUrl, $headers); + $filename = MediaFile::encodeFilename($original_name, $filehash); $fullpath = File::path($filename); - common_debug("StoreRemoteMedia retrieved file with id={$file->id} and will store in {$filename}"); + common_debug("StoreRemoteMedia retrieved url {$remoteUrl} for file with id={$file->id} " . + "and will store in {$fullpath}"); // Write the file to disk if it doesn't exist yet. Throw Exception on failure. - if (!file_exists($fullpath) && file_put_contents($fullpath, $imgData) === false) { + if ((!file_exists($fullpath) || substr($fullpath, 0, strlen(INSTALLDIR)) != INSTALLDIR) && + file_put_contents($fullpath, $imgData) === false) { throw new ServerException(_('Could not write downloaded file to disk.')); }