From 912e54cd7ac3225777808e3060171615f8478139 Mon Sep 17 00:00:00 2001 From: modmuss50 Date: Tue, 3 Jan 2023 14:11:16 +0000 Subject: [PATCH] Disallow insecure protocols for downloads. (#784) --- .../loom/util/download/DownloadBuilder.java | 20 +++++++++++++++++++ .../unit/download/DownloadFileTest.groovy | 9 +++++++++ .../unit/download/DownloadStringTest.groovy | 7 +++++++ .../test/unit/download/DownloadTest.groovy | 2 +- 4 files changed, 37 insertions(+), 1 deletion(-) diff --git a/src/main/java/net/fabricmc/loom/util/download/DownloadBuilder.java b/src/main/java/net/fabricmc/loom/util/download/DownloadBuilder.java index 15242936..0d3949a2 100644 --- a/src/main/java/net/fabricmc/loom/util/download/DownloadBuilder.java +++ b/src/main/java/net/fabricmc/loom/util/download/DownloadBuilder.java @@ -45,6 +45,7 @@ public class DownloadBuilder { private Duration maxAge = Duration.ZERO; private DownloadProgressListener progressListener = DownloadProgressListener.NONE; private int maxRetries = 3; + private boolean allowInsecureProtocol = false; private DownloadBuilder(URI url) { this.url = url; @@ -94,7 +95,16 @@ public class DownloadBuilder { return maxAge(ONE_DAY); } + public DownloadBuilder allowInsecureProtocol() { + this.allowInsecureProtocol = true; + return this; + } + private Download build() { + if (!allowInsecureProtocol && !isSecureUrl(url)) { + throw new IllegalArgumentException("Cannot create download for url (%s) with insecure protocol".formatted(url.toString())); + } + return new Download(this.url, this.expectedHash, this.useEtag, this.forceDownload, this.offline, maxAge, progressListener); } @@ -145,6 +155,16 @@ public class DownloadBuilder { throw new IllegalStateException(); } + // See comment on org.gradle.util.internal.GUtil.isSecureUrl + private static boolean isSecureUrl(URI url) { + if ("127.0.0.1".equals(url.getHost())) { + return true; + } + + final String scheme = url.getScheme(); + return !"http".equalsIgnoreCase(scheme); + } + @FunctionalInterface private interface DownloadSupplier { T get() throws DownloadException; diff --git a/src/test/groovy/net/fabricmc/loom/test/unit/download/DownloadFileTest.groovy b/src/test/groovy/net/fabricmc/loom/test/unit/download/DownloadFileTest.groovy index 6d7a408b..e3ee8d08 100644 --- a/src/test/groovy/net/fabricmc/loom/test/unit/download/DownloadFileTest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/unit/download/DownloadFileTest.groovy @@ -339,6 +339,15 @@ class DownloadFileTest extends DownloadTest { Files.readAllBytes(output) == data } + def "File: Insecure protocol"() { + setup: + def output = new File(File.createTempDir(), "file").toPath() + when: + def result = Download.create("http://fabricmc.net").downloadPath(output) + then: + thrown IllegalArgumentException + } + // Known def "Download Mojang Mappings"() { setup: diff --git a/src/test/groovy/net/fabricmc/loom/test/unit/download/DownloadStringTest.groovy b/src/test/groovy/net/fabricmc/loom/test/unit/download/DownloadStringTest.groovy index f28b20b2..319bff8f 100644 --- a/src/test/groovy/net/fabricmc/loom/test/unit/download/DownloadStringTest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/unit/download/DownloadStringTest.groovy @@ -109,4 +109,11 @@ class DownloadStringTest extends DownloadTest { then: result == "Hello World!" } + + def "String: Insecure protocol"() { + when: + def result = Download.create("http://fabricmc.net").downloadString() + then: + thrown IllegalArgumentException + } } diff --git a/src/test/groovy/net/fabricmc/loom/test/unit/download/DownloadTest.groovy b/src/test/groovy/net/fabricmc/loom/test/unit/download/DownloadTest.groovy index 36074642..b4e3981b 100644 --- a/src/test/groovy/net/fabricmc/loom/test/unit/download/DownloadTest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/unit/download/DownloadTest.groovy @@ -29,7 +29,7 @@ import spock.lang.Shared import spock.lang.Specification abstract class DownloadTest extends Specification { - static final String PATH = "http://localhost:9081" + static final String PATH = "http://127.0.0.1:9081" @Shared Javalin server = Javalin.create { config ->