Skip to content

Commit 9c0ce91

Browse files
committed
Enhance modpack content validation to prevent unsafe file paths and improve error logging
1 parent 78ae790 commit 9c0ce91

File tree

1 file changed

+32
-13
lines changed

1 file changed

+32
-13
lines changed

loader/core/src/main/java/pl/skidam/automodpack_loader_core/client/ModpackUtils.java

Lines changed: 32 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -550,26 +550,45 @@ private static Boolean askUserAboutCertificate(InetSocketAddress address, String
550550
return accepted.get();
551551
}
552552

553-
// check if modpackContent is valid/isn't malicious
554553
public static boolean potentiallyMalicious(Jsons.ModpackContentFields serverModpackContent) {
555-
String modpackName = serverModpackContent.modpackName;
556-
if (modpackName.contains("../") || modpackName.contains("/..")) {
557-
LOGGER.error("Modpack content is invalid, it contains /../ in modpack name");
554+
if (isUnsafePath(serverModpackContent.modpackName)) {
555+
LOGGER.error("Modpack content is invalid: modpack name '{}' is unsafe/malicious", serverModpackContent.modpackName);
558556
return true;
559557
}
560558

561-
for (var modpackContentItem : serverModpackContent.list) {
562-
String file = modpackContentItem.file.replace("\\", "/");
563-
if (file.contains("../") || file.contains("/..")) {
564-
LOGGER.error("Modpack content is invalid, it contains /../ in file name of {}", file);
565-
return true;
566-
}
559+
if (serverModpackContent.list == null || serverModpackContent.list.isEmpty()) {
560+
return false;
561+
}
567562

568-
String sha1 = modpackContentItem.sha1;
569-
if (sha1 == null || sha1.equals("null")) {
570-
LOGGER.error("Modpack content is invalid, it contains null sha1 in file of {}", file);
563+
return serverModpackContent.list.parallelStream().anyMatch(item -> {
564+
if (isUnsafePath(item.file)) {
565+
LOGGER.error("Modpack content is invalid: file path '{}' is unsafe/malicious", item.file);
571566
return true;
572567
}
568+
return false;
569+
});
570+
}
571+
572+
private static boolean isUnsafePath(String rawPath) {
573+
if (rawPath == null || rawPath.isBlank()) return true;
574+
575+
// Null Byte Check
576+
if (rawPath.indexOf('\0') != -1) return true;
577+
578+
// Most files are just "mods/fabric-api.jar", so they hit this and return false immediately
579+
if (!rawPath.contains("..")) {
580+
return false;
581+
}
582+
583+
// We must distinguish between malicious "../" and valid names like "super..mario.jar"
584+
String normalized = rawPath.replace('\\', '/');
585+
586+
// Edge case
587+
if (normalized.equals("..") || normalized.equals(".")) return true;
588+
589+
String[] segments = normalized.split("/");
590+
for (String segment : segments) {
591+
if (segment.equals("..")) return true; // Directory traversal
573592
}
574593

575594
return false;

0 commit comments

Comments
 (0)