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 7166260d4..63663b070 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.Metadata;
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 class FileTypeDetectUtils {
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 2c4351112..3a24fb342 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 org.springframework.web.util.UriUtils;
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 @@ class LocalAttachmentUploadHandler implements AttachmentHandler {
.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 @@ class LocalAttachmentUploadHandler implements AttachmentHandler {
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