From 24f8d7b57127e2607ec05adb7c912b3decddeb99 Mon Sep 17 00:00:00 2001 From: guqing <38999863+guqing@users.noreply.github.com> Date: Fri, 3 Jan 2025 17:32:13 +0800 Subject: [PATCH] fix: XSS vulnerability due to polyglot file type upload (#7149) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #### What type of PR is this? /kind bug /area core /milestone 2.20.x #### What this PR does / why we need it: 修复文件类型限制能通过混合文件类型绕过检测的问题 参考：https://github.com/halo-dev/halo/security/advisories/GHSA-99mc-ch53-pqh9 #### Does this PR introduce a user-facing change? ```release-note 修复文件类型限制能通过混合文件类型绕过检测的问题 ``` --- .../app/infra/utils/FileTypeDetectUtils.java | 38 +++++++++++++++++++ .../LocalAttachmentUploadHandler.java | 18 +++++++-- .../resources/config/i18n/messages.properties | 1 + .../config/i18n/messages_zh.properties | 1 + .../infra/utils/FileTypeDetectUtilsTest.java | 24 ++++++++++++ 5 files changed, 78 insertions(+), 4 deletions(-) diff --git a/api/src/main/java/run/halo/app/infra/utils/FileTypeDetectUtils.java b/api/src/main/java/run/halo/app/infra/utils/FileTypeDetectUtils.java index 7166260d4f..63663b0700 100644 --- a/api/src/main/java/run/halo/app/infra/utils/FileTypeDetectUtils.java +++ b/api/src/main/java/run/halo/app/infra/utils/FileTypeDetectUtils.java @@ -10,6 +10,7 @@ import org.apache.tika.metadata.TikaCoreProperties; import org.apache.tika.mime.MimeTypeException; import org.apache.tika.mime.MimeTypes; +import org.springframework.lang.NonNull; import org.springframework.util.Assert; @UtilityClass @@ -54,4 +55,41 @@ public static String detectFileExtension(String mimeType) throws MimeTypeExcepti MimeTypes mimeTypes = MimeTypes.getDefaultMimeTypes(); return mimeTypes.forName(mimeType).getExtension(); } + + /** + *

Get file extension from file name.

+ *

The obtained file extension is in lowercase and includes the dot, such as ".jpg".

+ */ + @NonNull + public static String getFileExtension(String fileName) { + Assert.notNull(fileName, "The fileName must not be null"); + int lastDot = fileName.lastIndexOf("."); + if (lastDot > 0) { + return fileName.substring(lastDot).toLowerCase(); + } + return ""; + } + + /** + *

Recommend to use this method to verify whether the file extension matches the file type + * after matching the file type to avoid XSS attacks such as bypassing detection by polyglot + * file

+ * + * @param mimeType file mime type,such as "image/png" + * @param fileName file name,such as "test.png" + * @see + * CVE Stored XSS + * @see gh-7149 + */ + public boolean isValidExtensionForMime(String mimeType, String fileName) { + Assert.notNull(mimeType, "The mimeType must not be null"); + Assert.notNull(fileName, "The fileName must not be null"); + String fileExtension = getFileExtension(fileName); + try { + String detectedExtByMime = detectFileExtension(mimeType); + return detectedExtByMime.equalsIgnoreCase(fileExtension); + } catch (MimeTypeException e) { + return false; + } + } } diff --git a/application/src/main/java/run/halo/app/core/attachment/endpoint/LocalAttachmentUploadHandler.java b/application/src/main/java/run/halo/app/core/attachment/endpoint/LocalAttachmentUploadHandler.java index 2c4351112f..3a24fb342d 100644 --- a/application/src/main/java/run/halo/app/core/attachment/endpoint/LocalAttachmentUploadHandler.java +++ b/application/src/main/java/run/halo/app/core/attachment/endpoint/LocalAttachmentUploadHandler.java @@ -36,6 +36,7 @@ import reactor.core.Exceptions; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; +import reactor.core.publisher.SynchronousSink; import reactor.core.scheduler.Schedulers; import reactor.util.retry.Retry; import run.halo.app.core.attachment.AttachmentRootGetter; @@ -159,6 +160,10 @@ private Mono validateFile(FilePart file, PolicySetting setting) { .next() .handle((dataBuffer, sink) -> { var mimeType = detectMimeType(dataBuffer.asInputStream(), file.name()); + if (!FileTypeDetectUtils.isValidExtensionForMime(mimeType, file.name())) { + handleFileTypeError(sink, "fileTypeNotMatch", mimeType); + return; + } var isAllow = setting.getAllowedFileTypes() .stream() .map(FileCategoryMatcher::of) @@ -167,16 +172,21 @@ private Mono validateFile(FilePart file, PolicySetting setting) { sink.next(dataBuffer); return; } - sink.error(new FileTypeNotAllowedException("File type is not allowed", - "problemDetail.attachment.upload.fileTypeNotSupported", - new Object[] {mimeType}) - ); + handleFileTypeError(sink, "fileTypeNotSupported", mimeType); }); validations.add(typeValidator); } return Mono.when(validations); } + private static void handleFileTypeError(SynchronousSink sink, String detailCode, + String mimeType) { + sink.error(new FileTypeNotAllowedException("File type is not allowed", + "problemDetail.attachment.upload." + detailCode, + new Object[] {mimeType}) + ); + } + @NonNull private String detectMimeType(InputStream inputStream, String name) { try { diff --git a/application/src/main/resources/config/i18n/messages.properties b/application/src/main/resources/config/i18n/messages.properties index 565d813987..81e36aa05b 100644 --- a/application/src/main/resources/config/i18n/messages.properties +++ b/application/src/main/resources/config/i18n/messages.properties @@ -82,6 +82,7 @@ problemDetail.conflict=Conflict detected, please check the data and retry. problemDetail.migration.backup.notFound=The backup file does not exist or has been deleted. problemDetail.attachment.upload.fileSizeExceeded=Make sure the file size is less than {0}. problemDetail.attachment.upload.fileTypeNotSupported=Unsupported upload of {0} type files. +problemDetail.attachment.upload.fileTypeNotMatch=The file type {0} does not match the file extension, and the upload is rejected. problemDetail.comment.waitingForApproval=Comment is awaiting approval. title.visibility.identification.private=(Private) diff --git a/application/src/main/resources/config/i18n/messages_zh.properties b/application/src/main/resources/config/i18n/messages_zh.properties index 767d79b77e..bdab92ac74 100644 --- a/application/src/main/resources/config/i18n/messages_zh.properties +++ b/application/src/main/resources/config/i18n/messages_zh.properties @@ -55,6 +55,7 @@ problemDetail.conflict=检测到冲突，请检查数据后重试。 problemDetail.migration.backup.notFound=备份文件不存在或已删除。 problemDetail.attachment.upload.fileSizeExceeded=最大支持上传 {0} 大小的文件。 problemDetail.attachment.upload.fileTypeNotSupported=不支持上传 {0} 类型的文件。 +problemDetail.attachment.upload.fileTypeNotMatch=文件类型 {0} 与文件扩展名不匹配，上传被拒绝。 problemDetail.comment.waitingForApproval=评论审核中。 title.visibility.identification.private=（私有） diff --git a/application/src/test/java/run/halo/app/infra/utils/FileTypeDetectUtilsTest.java b/application/src/test/java/run/halo/app/infra/utils/FileTypeDetectUtilsTest.java index 1c3e407d08..8cf0b4fe0e 100644 --- a/application/src/test/java/run/halo/app/infra/utils/FileTypeDetectUtilsTest.java +++ b/application/src/test/java/run/halo/app/infra/utils/FileTypeDetectUtilsTest.java @@ -96,5 +96,29 @@ void detectFileExtensionTest() throws MimeTypeException { ext = FileTypeDetectUtils.detectFileExtension("application/zip"); assertThat(ext).isEqualTo(".zip"); + + ext = FileTypeDetectUtils.detectFileExtension("image/bmp"); + assertThat(ext).isEqualTo(".bmp"); + } + + @Test + void getFileExtensionTest() { + var ext = FileTypeDetectUtils.getFileExtension("BMP+HTML+JAR.html"); + assertThat(ext).isEqualTo(".html"); + + ext = FileTypeDetectUtils.getFileExtension("test.jpg"); + assertThat(ext).isEqualTo(".jpg"); + + ext = FileTypeDetectUtils.getFileExtension("hello"); + assertThat(ext).isEqualTo(""); + } + + @Test + void isValidExtensionForMimeTest() { + assertThat(FileTypeDetectUtils.isValidExtensionForMime("image/bmp", "hello.html")) + .isFalse(); + + assertThat(FileTypeDetectUtils.isValidExtensionForMime("image/bmp", "hello.bmp")) + .isTrue(); } }