Skip to content

Commit

Permalink
fix: XSS vulnerability due to polyglot file type upload
Browse files Browse the repository at this point in the history
  • Loading branch information
guqing committed Dec 18, 2024
1 parent 156a304 commit 59b51eb
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -54,4 +55,41 @@ public static String detectFileExtension(String mimeType) throws MimeTypeExcepti
MimeTypes mimeTypes = MimeTypes.getDefaultMimeTypes();
return mimeTypes.forName(mimeType).getExtension();
}

/**
* <p>Get file extension from file name.</p>
* <p>The obtained file extension is in lowercase and includes the dot, such as ".jpg".</p>
*/
@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 "";
}

/**
* <p>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</p>
*
* @param mimeType file mime type,such as "image/png"
* @param fileName file name,such as "test.png"
* @see
* <a href="https://github.com/halo-dev/halo/security/advisories/GHSA-99mc-ch53-pqh9">CVE Stored XSS</a>
* @see <a href="https://github.com/halo-dev/halo/pull/7149">gh-7149</a>
*/
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;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -159,6 +160,10 @@ private Mono<Void> 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)
Expand All @@ -167,16 +172,21 @@ private Mono<Void> 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<Object> 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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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=(私有)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}

0 comments on commit 59b51eb

Please sign in to comment.