From cfca519c5a754be83e21e0966e04b6aaa7ef4107 Mon Sep 17 00:00:00 2001 From: Yuri Kuznetsov Date: Fri, 1 Jul 2022 19:52:50 +0300 Subject: [PATCH] image mime type detection --- application/Espo/Controllers/Attachment.php | 22 ++++++ application/Espo/Core/Utils/File/MimeType.php | 18 +++-- application/Espo/Services/Attachment.php | 67 ++++++++++++++----- composer.json | 3 +- 4 files changed, 87 insertions(+), 23 deletions(-) diff --git a/application/Espo/Controllers/Attachment.php b/application/Espo/Controllers/Attachment.php index 89e835864d..8c2db6b1f5 100644 --- a/application/Espo/Controllers/Attachment.php +++ b/application/Espo/Controllers/Attachment.php @@ -52,6 +52,11 @@ class Attachment extends RecordBase return parent::getActionList($request, $response); } + /** + * @throws BadRequest + * @throws Forbidden + * @throws \Espo\Core\Exceptions\Error + */ public function postActionGetAttachmentFromImageUrl(Request $request): stdClass { $data = $request->getParsedBody(); @@ -69,6 +74,12 @@ class Attachment extends RecordBase ->getValueMap(); } + /** + * @throws BadRequest + * @throws Forbidden + * @throws \Espo\Core\Exceptions\Error + * @throws \Espo\Core\Exceptions\NotFound + */ public function postActionGetCopiedAttachment(Request $request): stdClass { $data = $request->getParsedBody(); @@ -86,6 +97,11 @@ class Attachment extends RecordBase ->getValueMap(); } + /** + * @throws BadRequest + * @throws Forbidden + * @throws \Espo\Core\Exceptions\NotFound + */ public function getActionFile(Request $request, Response $response): void { $id = $request->getRouteParam('id'); @@ -103,6 +119,12 @@ class Attachment extends RecordBase ->setBody($fileData->stream); } + /** + * @throws BadRequest + * @throws Forbidden + * @throws \Espo\Core\Exceptions\Error + * @throws \Espo\Core\Exceptions\NotFound + */ public function postActionChunk(Request $request, Response $response): void { $id = $request->getRouteParam('id'); diff --git a/application/Espo/Core/Utils/File/MimeType.php b/application/Espo/Core/Utils/File/MimeType.php index 80cb513927..530beac639 100644 --- a/application/Espo/Core/Utils/File/MimeType.php +++ b/application/Espo/Core/Utils/File/MimeType.php @@ -40,17 +40,21 @@ class MimeType $this->metadata = $metadata; } - public function getMimeTypeByExtension(string $extension): ?string + /** + * @return string[] + */ + public function getMimeTypeListByExtension(string $extension): array { $extensionLowerCase = strtolower($extension); - /** @var string[]|null */ - $typeList = $this->metadata - ->get(['app', 'file', 'extensionMimeTypeMap', $extensionLowerCase]); + /** @var string[] */ + return $this->metadata + ->get(['app', 'file', 'extensionMimeTypeMap', $extensionLowerCase]) ?? []; + } - if ($typeList === null) { - return null; - } + public function getMimeTypeByExtension(string $extension): ?string + { + $typeList = $this->getMimeTypeListByExtension($extension); return $typeList[0] ?? null; } diff --git a/application/Espo/Services/Attachment.php b/application/Espo/Services/Attachment.php index f4b0f55144..e48e464e81 100644 --- a/application/Espo/Services/Attachment.php +++ b/application/Espo/Services/Attachment.php @@ -98,24 +98,24 @@ class Attachment extends Record private const FIELD_TYPE_WYSIWYG = 'wysiwyg'; /** - * @param string $fileData * @throws Forbidden + * @todo Check where is it used. Maybe needs to be removed. */ - public function upload($fileData): Entity + public function upload(string $fileData): Entity { if (!$this->acl->checkScope('Attachment', Table::ACTION_CREATE)) { throw new Forbidden(); } + $contents = ''; + $arr = explode(',', $fileData); if (count($arr) > 1) { list($prefix, $contents) = $arr; + $contents = base64_decode($contents); } - else { - $contents = ''; - } $attachment = $this->entityManager->getNewEntity('Attachment'); @@ -306,11 +306,10 @@ class Attachment extends Record return; } - $name = $attachment->getName() ?? ''; + $extension = self::getFileExtension($attachment) ?? null; - $extension = strtolower( - array_slice(explode('.', $name), -1)[0] ?? '' - ); + $mimeType = $this->getMimeTypeUtil()->getMimeTypeByExtension($extension) ?? + $attachment->getType(); /** @var string[] */ $accept = $this->metadata->get(['entityDefs', $entityType, 'fields', $field, 'accept']) ?? []; @@ -319,9 +318,6 @@ class Attachment extends Record return; } - $mimeType = $this->getMimeTypeUtil()->getMimeTypeByExtension($extension) ?? - $attachment->getType(); - $found = false; foreach ($accept as $token) { @@ -348,9 +344,7 @@ class Attachment extends Record */ private function checkAttachmentTypeImage(AttachmentEntity $attachment): void { - $name = $attachment->getName() ?? ''; - - $extension = array_slice(explode('.', $name), -1)[0] ?? ''; + $extension = self::getFileExtension($attachment) ?? null; $mimeType = $this->getMimeTypeUtil()->getMimeTypeByExtension($extension); @@ -360,6 +354,49 @@ class Attachment extends Record if (!in_array($mimeType, $imageTypeList)) { throw new ForbiddenSilent("Not allowed file type."); } + + $setMimeType = $attachment->getType(); + + if (strtolower($setMimeType ?? '') !== $mimeType) { + throw new ForbiddenSilent("Passed type does not correspond to extension."); + } + + $this->checkDetectedMimeType($attachment); + } + + private static function getFileExtension(AttachmentEntity $attachment): ?string + { + $name = $attachment->getName() ?? ''; + + return array_slice(explode('.', $name), -1)[0] ?? null; + } + + /** + * @throws Forbidden + */ + private function checkDetectedMimeType(AttachmentEntity $attachment): void + { + // ext-fileinfo required, otherwise bypass. + if (!class_exists('\finfo') || !defined('FILEINFO_MIME_TYPE')) { + return; + } + + /** @var string|null */ + $contents = $attachment->get('contents'); + + if (!$contents) { + return; + } + + $extension = self::getFileExtension($attachment) ?? null; + + $mimeTypeList = $this->getMimeTypeUtil()->getMimeTypeListByExtension($extension); + + $detectedMimeType = (new \finfo(FILEINFO_MIME_TYPE))->buffer($contents); + + if (!in_array($detectedMimeType, $mimeTypeList)) { + throw new ForbiddenSilent("Detected mime type does not correspond to extension."); + } } /** diff --git a/composer.json b/composer.json index 9687bf3b85..d3493b4cee 100644 --- a/composer.json +++ b/composer.json @@ -52,7 +52,8 @@ "suggest": { "ext-bcmath": "*", "ext-zmq": "*", - "ext-ldap": "*" + "ext-ldap": "*", + "ext-fileinfo": "*" }, "autoload": { "psr-4": {