TDLM/HMCL
3
0 Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases 6 Wiki Activity

fix: harden MultiMC/authlib/IO trust boundaries (#8–#13) #14

Merged
layue13 merged 1 commits from cursor/fix-open-security-issues-1729 into main 2026-11-09 21:19:55 +00:00
Conversation 0 Commits 1 Files Changed 14
+268 -30
14 changed files with 268 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

HMCL/src/main/java/org/jackhuang/hmcl/game/ModpackHelper.java
+7 -17
View File
@@ -19,7 +19,6 @@ import org.jackhuang.hmcl.modpack.server.ServerModpackRemoteInstallTask;
import org.jackhuang.hmcl.setting.*;
import org.jackhuang.hmcl.task.Schedulers;
import org.jackhuang.hmcl.task.Task;
import org.jackhuang.hmcl.util.Lang;
import org.jackhuang.hmcl.util.PortablePath;
import org.jackhuang.hmcl.util.function.ExceptionalConsumer;
import org.jackhuang.hmcl.util.function.ExceptionalRunnable;
@@ -44,6 +43,7 @@ import java.util.Set;
import static org.jackhuang.hmcl.util.Lang.mapOf;
import static org.jackhuang.hmcl.util.Pair.pair;
import static org.jackhuang.hmcl.util.logging.Logger.LOG;
/// Utilities for reading, installing, and applying modpack-specific game settings.
@NotNullByDefault
@@ -314,18 +314,13 @@ public final class ModpackHelper {
setting.minMemoryProperty().setValue(c.getMinMemory());
}
// MultiMC packs are untrusted input: never auto-apply PreLaunch/Wrapper/JVM overrides.
if (c.isOverrideCommands()) {
setting.getOverrideProperties().addAll(List.of(
GameSettings.PROPERTY_COMMAND_WRAPPER,
GameSettings.PROPERTY_PRE_LAUNCH_COMMAND
));
setting.commandWrapperProperty().setValue(Objects.requireNonNullElse(c.getWrapperCommand(), ""));
setting.preLaunchCommandProperty().setValue(Objects.requireNonNullElse(c.getPreLaunchCommand(), ""));
LOG.warning("Ignoring MultiMC OverrideCommands (PreLaunch/Wrapper) from modpack for security");
}
if (c.isOverrideJavaArgs()) {
setting.getOverrideProperties().add(GameSettings.PROPERTY_JVM_OPTIONS);
setting.jvmOptionsProperty().setValue(Objects.requireNonNullElse(c.getJvmArgs(), ""));
LOG.warning("Ignoring MultiMC OverrideJavaArgs from modpack for security");
}
if (c.isOverrideConsole()) {
@@ -348,18 +343,13 @@ public final class ModpackHelper {
}
private static void applyCommandAndJvmSettings(MultiMCInstanceConfiguration c, GameSettings.Instance setting) {
// Same policy as toGameSettings: do not import executable commands / raw JVM args from packs.
if (c.isOverrideCommands()) {
setting.getOverrideProperties().addAll(List.of(
GameSettings.PROPERTY_COMMAND_WRAPPER,
GameSettings.PROPERTY_PRE_LAUNCH_COMMAND
));
setting.commandWrapperProperty().setValue(Lang.nonNull(c.getWrapperCommand(), ""));
setting.preLaunchCommandProperty().setValue(Lang.nonNull(c.getPreLaunchCommand(), ""));
LOG.warning("Ignoring MultiMC OverrideCommands (PreLaunch/Wrapper) from modpack update for security");
}
if (c.isOverrideJavaArgs()) {
setting.getOverrideProperties().add(GameSettings.PROPERTY_JVM_OPTIONS);
setting.jvmOptionsProperty().setValue(Lang.nonNull(c.getJvmArgs(), ""));
LOG.warning("Ignoring MultiMC OverrideJavaArgs from modpack update for security");
}
}
HMCL/src/main/java/org/jackhuang/hmcl/ui/FXUtils.java
+21
View File
@@ -602,6 +602,10 @@ public final class FXUtils {
return;
String url = NetworkUtils.encodeLocation(link);
if (!isAllowedOpenLink(url)) {
LOG.warning("Refusing to open link with disallowed scheme: " + link);
return;
}
thread(() -> {
try {
if (OperatingSystem.CURRENT_OS == OperatingSystem.WINDOWS) {
@@ -635,6 +639,23 @@ public final class FXUtils {
});
}
/// Only {@code http}/{@code https}/{@code mailto} may be handed to the OS URL handler.
static boolean isAllowedOpenLink(String link) {
try {
URI uri = new URI(link);
String scheme = uri.getScheme();
if (scheme == null) {
return false;
}
return switch (scheme.toLowerCase(java.util.Locale.ROOT)) {
case "http", "https", "mailto" -> true;
default -> false;
};
} catch (Exception e) {
return false;
}
}
public static <T> void bind(JFXTextField textField, Property<T> property, StringConverter<T> converter) {
TextFieldBinding<T> binding = new TextFieldBinding<>(textField, property, converter);
binding.updateTextField();
HMCLBoot/src/main/java/org/jackhuang/hmcl/DesktopUtils.java
+20
View File
@@ -42,6 +42,11 @@ public final class DesktopUtils {
}
public static void openLink(String link) {
if (!isAllowedOpenLink(link)) {
System.err.println("Refusing to open link with disallowed scheme: " + link);
return;
}
String osName = System.getProperty("os.name").toLowerCase(Locale.ROOT);
Throwable e1 = null;
@@ -79,6 +84,21 @@ public final class DesktopUtils {
}
}
/// Only {@code http}/{@code https}/{@code mailto} may be handed to the OS URL handler.
static boolean isAllowedOpenLink(String link) {
try {
URI uri = new URI(link);
String scheme = uri.getScheme();
if (scheme == null) {
return false;
}
String lower = scheme.toLowerCase(Locale.ROOT);
return "http".equals(lower) || "https".equals(lower) || "mailto".equals(lower);
} catch (Exception e) {
return false;
}
}
private DesktopUtils() {
}
}
HMCLCore/src/main/java/org/jackhuang/hmcl/auth/authlibinjector/AuthlibInjectorServer.java
+11 -3
View File
@@ -45,10 +45,18 @@ public class AuthlibInjectorServer implements Observable {
String ali = conn.getHeaderField("x-authlib-injector-api-location");
if (ali != null) {
WebURL absoluteAli = WebURL.parse(ali, WebURL.of(conn.getURL()));
// Only follow same-host HTTPS redirects; open redirects to other hosts are rejected.
if (!urlEqualsIgnoreSlash(webURL.toString(), absoluteAli.toString())) {
conn.disconnect();
url = absoluteAli.toString();
conn = NetworkUtils.resolveConnection(NetworkUtils.createHttpConnection(absoluteAli));
if (!"https".equalsIgnoreCase(absoluteAli.getScheme())
|| absoluteAli.getHost() == null
|| webURL.getHost() == null
|| !absoluteAli.getHost().equalsIgnoreCase(webURL.getHost())) {
LOG.warning("Ignoring x-authlib-injector-api-location redirect outside original HTTPS host: " + absoluteAli);
} else {
conn.disconnect();
url = absoluteAli.toString();
conn = NetworkUtils.resolveConnection(NetworkUtils.createHttpConnection(absoluteAli));
}
}
}
HMCLCore/src/main/java/org/jackhuang/hmcl/modpack/multimc/MultiMCInstancePatch.java
+2 -1
View File
@@ -7,6 +7,7 @@ import com.google.gson.annotations.SerializedName;
import org.jackhuang.hmcl.game.*;
import org.jackhuang.hmcl.util.Immutable;
import org.jackhuang.hmcl.util.JvmArgumentFilter;
import org.jackhuang.hmcl.util.Lang;
import org.jackhuang.hmcl.util.StringUtils;
import org.jackhuang.hmcl.util.gson.JsonUtils;
@@ -282,7 +283,7 @@ public final class MultiMCInstancePatch {
if (minecraftArguments == null & patch.getMinecraftArguments() != null) {
minecraftArguments = StringUtils.tokenize(patch.getMinecraftArguments());
}
for (String jvmArg : patch.getJvmArgs()) {
for (String jvmArg : JvmArgumentFilter.filter(patch.getJvmArgs())) {
jvmArguments.add(new StringArgument(jvmArg));
}
mainClass = Lang.requireNonNullElse(mainClass, patch.getMainClass());
HMCLCore/src/main/java/org/jackhuang/hmcl/modpack/server/Lwjgl3ifySupport.java
+2 -1
View File
@@ -5,6 +5,7 @@ package org.jackhuang.hmcl.modpack.server;
import org.jackhuang.hmcl.game.LaunchOptions;
import org.jackhuang.hmcl.modpack.multimc.MultiMCInstancePatch;
import org.jackhuang.hmcl.modpack.multimc.MultiMCManifest;
import org.jackhuang.hmcl.util.JvmArgumentFilter;
import org.jackhuang.hmcl.util.gson.JsonUtils;
import org.jetbrains.annotations.NotNullByDefault;
@@ -55,7 +56,7 @@ public final class Lwjgl3ifySupport {
try {
MultiMCInstancePatch patch = MultiMCInstancePatch.read(
patchFile.getFileName().toString(), Files.readString(patchFile));
args.addAll(patch.getJvmArgs());
args.addAll(JvmArgumentFilter.filter(patch.getJvmArgs()));
} catch (IOException | RuntimeException e) {
LOG.warning("Failed to read lwjgl3ify patch " + patchFile, e);
}
HMCLCore/src/main/java/org/jackhuang/hmcl/util/JvmArgumentFilter.java
+57
View File
@@ -0,0 +1,57 @@
// SPDX-FileCopyrightText: Copyright (C) 2026 The HMCL Authors
// SPDX-License-Identifier: GPL-3.0-or-later WITH AdditionRef-HMCL
package org.jackhuang.hmcl.util;
import org.jetbrains.annotations.Nullable;
import java.util.ArrayList;
import java.util.List;
import java.util.Locale;
import static org.jackhuang.hmcl.util.logging.Logger.LOG;
/**
* Filters JVM arguments coming from untrusted modpack / MultiMC patch input.
*
* <p>Denies agent injection and {@code -XX:On*Error} command hooks while allowing ordinary
* memory / GC / system-property args used by companions such as lwjgl3ify.
*/
public final class JvmArgumentFilter {
private JvmArgumentFilter() {
}
/// Returns whether a single JVM argument is safe to accept from modpack / patch metadata.
public static boolean isAllowed(@Nullable String arg) {
if (arg == null) {
return false;
}
String trimmed = arg.trim();
if (trimmed.isEmpty()) {
return false;
}
String lower = trimmed.toLowerCase(Locale.ROOT);
if (lower.startsWith("-javaagent")
|| lower.startsWith("-agentpath")
|| lower.startsWith("-agentlib")
|| lower.startsWith("-xbootclasspath")
|| lower.startsWith("-xx:onoutofmemoryerror")
|| lower.startsWith("-xx:onerror")
|| lower.startsWith("-xx:exitonoutofmemoryerror")) {
return false;
}
return true;
}
/// Copies {@code args}, dropping disallowed entries and logging each skip.
public static List<String> filter(Iterable<String> args) {
List<String> out = new ArrayList<>();
for (String arg : args) {
if (isAllowed(arg)) {
out.add(arg);
} else {
LOG.warning("Ignoring disallowed JVM argument from modpack/patch: " + arg);
}
}
return out;
}
}
HMCLCore/src/main/java/org/jackhuang/hmcl/util/io/FileUtils.java
+10 -2
View File
@@ -306,6 +306,7 @@ public final class FileUtils {
}
public static void copyDirectory(Path src, Path dest, Predicate<String> filePredicate) throws IOException {
Path destRoot = dest.toAbsolutePath().normalize();
Files.walkFileTree(src, new SimpleFileVisitor<>() {
@Override
public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException {
@@ -313,7 +314,11 @@ public final class FileUtils {
return FileVisitResult.SKIP_SUBTREE;
}
Path destFile = dest.resolve(src.relativize(file).toString());
Path destFile = destRoot.resolve(src.relativize(file).toString()).toAbsolutePath().normalize();
if (!destFile.startsWith(destRoot)) {
throw new IOException("Refusing to copy outside destination directory: " + file);
}
Files.createDirectories(destFile.getParent());
Files.copy(file, destFile, StandardCopyOption.REPLACE_EXISTING);
return FileVisitResult.CONTINUE;
}
@@ -324,7 +329,10 @@ public final class FileUtils {
return FileVisitResult.SKIP_SUBTREE;
}
Path destDir = dest.resolve(src.relativize(dir).toString());
Path destDir = destRoot.resolve(src.relativize(dir).toString()).toAbsolutePath().normalize();
if (!destDir.startsWith(destRoot)) {
throw new IOException("Refusing to create directory outside destination: " + dir);
}
Files.createDirectories(destDir);
return FileVisitResult.CONTINUE;
}
HMCLCore/src/main/java/org/jackhuang/hmcl/util/io/HttpRequest.java
+4 -2
View File
@@ -10,6 +10,7 @@ import org.jackhuang.hmcl.util.function.ExceptionalSupplier;
import org.jackhuang.hmcl.util.gson.JsonUtils;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.net.HttpURLConnection;
import java.net.MalformedURLException;
@@ -134,9 +135,10 @@ public abstract class HttpRequest {
con = resolveConnection(con);
checkResponseCode(con);
return IOUtils.readFullyAsString("gzip".equals(con.getContentEncoding())
InputStream body = "gzip".equals(con.getContentEncoding())
? IOUtils.wrapFromGZip(con.getInputStream())
: con.getInputStream());
: con.getInputStream();
return IOUtils.readFullyAsString(body, UTF_8, IOUtils.DEFAULT_MAX_HTTP_BODY_BYTES);
}, retryTimes);
}
}
HMCLCore/src/main/java/org/jackhuang/hmcl/util/io/IOUtils.java
+36 -2
View File
@@ -5,7 +5,13 @@ package org.jackhuang.hmcl.util.io;
import kala.encdet.EncodingDetector;
import org.jetbrains.annotations.Nullable;
import java.io.*;
import java.io.BufferedReader;
import java.io.ByteArrayOutputStream;
import java.io.EOFException;
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
import java.io.OutputStream;
import java.nio.ByteBuffer;
import java.nio.channels.Channels;
import java.nio.channels.FileChannel;
@@ -29,6 +35,9 @@ public final class IOUtils {
public static final int DEFAULT_BUFFER_SIZE = 32 * 1024;
/// Soft cap for HTTP / metadata bodies read into memory (authlib, API JSON, etc.).
public static final long DEFAULT_MAX_HTTP_BODY_BYTES = 16L * 1024L * 1024L;
public static BufferedReader newBufferedReaderMaybeNativeEncoding(Path file) throws IOException {
if (NATIVE_CHARSET == UTF_8)
return Files.newBufferedReader(file);
@@ -68,8 +77,29 @@ public final class IOUtils {
}
public static byte[] readFully(InputStream stream) throws IOException {
return readFully(stream, -1L);
}
/// Reads the stream fully, optionally refusing bodies larger than {@code maxBytes}.
///
/// @param maxBytes maximum accepted size, or {@code <= 0} for no limit
public static byte[] readFully(InputStream stream, long maxBytes) throws IOException {
try (stream) {
return stream.readAllBytes();
if (maxBytes <= 0) {
return stream.readAllBytes();
}
ByteArrayOutputStream out = new ByteArrayOutputStream(Math.min(DEFAULT_BUFFER_SIZE, (int) Math.min(maxBytes, Integer.MAX_VALUE)));
byte[] buf = new byte[DEFAULT_BUFFER_SIZE];
long total = 0;
int n;
while ((n = stream.read(buf)) >= 0) {
total += n;
if (total > maxBytes) {
throw new IOException("Response body exceeds limit of " + maxBytes + " bytes");
}
out.write(buf, 0, n);
}
return out.toByteArray();
}
}
@@ -81,6 +111,10 @@ public final class IOUtils {
return new String(readFully(stream), charset);
}
public static String readFullyAsString(InputStream stream, Charset charset, long maxBytes) throws IOException {
return new String(readFully(stream, maxBytes), charset);
}
public static void skipNBytes(InputStream input, long n) throws IOException {
while (n > 0) {
long ns = input.skip(n);
HMCLCore/src/main/java/org/jackhuang/hmcl/util/io/NetworkUtils.java
+2 -2
View File
@@ -437,13 +437,13 @@ public final class NetworkUtils {
Charset charset = getCharsetFromContentType(con.getHeaderField("Content-Type"));
try (InputStream stdout = con.getInputStream()) {
return IOUtils.readFullyAsString(contentEncoding.wrap(stdout), charset);
return IOUtils.readFullyAsString(contentEncoding.wrap(stdout), charset, IOUtils.DEFAULT_MAX_HTTP_BODY_BYTES);
} catch (IOException e) {
if (con instanceof HttpURLConnection) {
try (InputStream stderr = ((HttpURLConnection) con).getErrorStream()) {
if (stderr == null)
throw e;
return IOUtils.readFullyAsString(contentEncoding.wrap(stderr), charset);
return IOUtils.readFullyAsString(contentEncoding.wrap(stderr), charset, IOUtils.DEFAULT_MAX_HTTP_BODY_BYTES);
}
} else {
throw e;
HMCLCore/src/test/java/org/jackhuang/hmcl/util/JvmArgumentFilterTest.java
+34
View File
@@ -0,0 +1,34 @@
// SPDX-FileCopyrightText: Copyright (C) 2026 The HMCL Authors
// SPDX-License-Identifier: GPL-3.0-or-later WITH AdditionRef-HMCL
package org.jackhuang.hmcl.util;
import org.junit.jupiter.api.Test;
import java.util.List;
import static org.junit.jupiter.api.Assertions.*;
public class JvmArgumentFilterTest {
@Test
public void allowsOrdinaryArgs() {
assertTrue(JvmArgumentFilter.isAllowed("-Xmx2G"));
assertTrue(JvmArgumentFilter.isAllowed("-Djava.library.path=/tmp"));
assertTrue(JvmArgumentFilter.isAllowed("--add-opens=java.base/java.lang=ALL-UNNAMED"));
}
@Test
public void rejectsAgentsAndOnErrorHooks() {
assertFalse(JvmArgumentFilter.isAllowed("-javaagent:/tmp/evil.jar"));
assertFalse(JvmArgumentFilter.isAllowed("-agentpath:/tmp/evil.so"));
assertFalse(JvmArgumentFilter.isAllowed("-agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=*:5005"));
assertFalse(JvmArgumentFilter.isAllowed("-Xbootclasspath/a:/tmp/evil.jar"));
assertFalse(JvmArgumentFilter.isAllowed("-XX:OnOutOfMemoryError=curl evil.example"));
assertFalse(JvmArgumentFilter.isAllowed("-XX:OnError=id"));
}
@Test
public void filterDropsDisallowed() {
assertEquals(List.of("-Xmx1G"),
JvmArgumentFilter.filter(List.of("-Xmx1G", "-javaagent:x.jar")));
}
}
HMCLCore/src/test/java/org/jackhuang/hmcl/util/io/CopyDirectoryTraversalTest.java
+36
View File
@@ -0,0 +1,36 @@
// SPDX-FileCopyrightText: Copyright (C) 2026 The HMCL Authors
// SPDX-License-Identifier: GPL-3.0-or-later WITH AdditionRef-HMCL
package org.jackhuang.hmcl.util.io;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import static org.junit.jupiter.api.Assertions.*;
public class CopyDirectoryTraversalTest {
@TempDir
Path temp;
@Test
public void copiesWithinDestination() throws IOException {
Path src = temp.resolve("src");
Path dest = temp.resolve("dest");
Files.createDirectories(src.resolve("a"));
Files.writeString(src.resolve("a/b.txt"), "hi");
FileUtils.copyDirectory(src, dest);
assertEquals("hi", Files.readString(dest.resolve("a/b.txt")));
}
@Test
public void destinationRootGuardMatchesUnzipperStyle() {
Path destRoot = temp.resolve("dest").toAbsolutePath().normalize();
Path escape = destRoot.resolve("sub/../../outside.txt").toAbsolutePath().normalize();
assertFalse(escape.startsWith(destRoot), "normalize + startsWith must reject escapes");
Path inside = destRoot.resolve("sub/file.txt").toAbsolutePath().normalize();
assertTrue(inside.startsWith(destRoot));
}
}
HMCLCore/src/test/java/org/jackhuang/hmcl/util/io/IOUtilsReadLimitTest.java
+26
View File
@@ -0,0 +1,26 @@
// SPDX-FileCopyrightText: Copyright (C) 2026 The HMCL Authors
// SPDX-License-Identifier: GPL-3.0-or-later WITH AdditionRef-HMCL
package org.jackhuang.hmcl.util.io;
import org.junit.jupiter.api.Test;
import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import static org.junit.jupiter.api.Assertions.*;
public class IOUtilsReadLimitTest {
@Test
public void rejectsOversizedBody() {
byte[] data = new byte[100];
assertThrows(IOException.class, () -> IOUtils.readFully(new ByteArrayInputStream(data), 50));
}
@Test
public void acceptsWithinLimit() throws IOException {
byte[] data = "hello".getBytes(StandardCharsets.UTF_8);
assertArrayEquals(data, IOUtils.readFully(new ByteArrayInputStream(data), 16));
assertEquals("hello", IOUtils.readFullyAsString(new ByteArrayInputStream(data), StandardCharsets.UTF_8, 16));
}
}