diff --git a/pkl-core/src/main/java/org/pkl/core/packages/PackageResolvers.java b/pkl-core/src/main/java/org/pkl/core/packages/PackageResolvers.java index 5cceb215b..b2baefe78 100644 --- a/pkl-core/src/main/java/org/pkl/core/packages/PackageResolvers.java +++ b/pkl-core/src/main/java/org/pkl/core/packages/PackageResolvers.java @@ -45,6 +45,7 @@ import java.util.stream.StreamSupport; import java.util.zip.ZipInputStream; import org.graalvm.collections.EconomicMap; import org.jspecify.annotations.Nullable; +import org.pkl.core.PklBugException; import org.pkl.core.SecurityManager; import org.pkl.core.SecurityManagerException; import org.pkl.core.http.HttpClient; @@ -456,10 +457,17 @@ final class PackageResolvers { } private Path getRelativePath(PackageUri uri) { - return Path.of( - CACHE_DIR_PREFIX, - IoUtils.encodePath(uri.getUri().getAuthority()), - getEffectivePackageUriPath(uri)); + var relativePath = + Path.of( + CACHE_DIR_PREFIX, + IoUtils.encodePath(uri.getUri().getAuthority()), + getEffectivePackageUriPath(uri)); + // ensure the derived path cannot escape the cache directory + var resolved = cacheDir.resolve(relativePath).normalize(); + if (!resolved.startsWith(cacheDir.normalize())) { + throw new PklBugException("Package URI escapes the cache directory"); + } + return relativePath; } private String getLastSegmentName(PackageUri packageUri) { diff --git a/pkl-core/src/main/java/org/pkl/core/packages/PackageUri.java b/pkl-core/src/main/java/org/pkl/core/packages/PackageUri.java index 2e409a5ce..c860c6dc0 100644 --- a/pkl-core/src/main/java/org/pkl/core/packages/PackageUri.java +++ b/pkl-core/src/main/java/org/pkl/core/packages/PackageUri.java @@ -61,6 +61,13 @@ public final class PackageUri { throw new URISyntaxException( uri.toString(), ErrorMessages.create("missingPathInPackageUri", uri)); } + // reject `..` segments, percent-encoded or not + for (var segment : path.split("/", -1)) { + if (segment.equals("..")) { + throw new URISyntaxException( + uri.toString(), ErrorMessages.create("invalidRelativePathInPackageUri")); + } + } var versionIdx = path.lastIndexOf('@'); if (versionIdx == -1) { throw new URISyntaxException( diff --git a/pkl-core/src/main/resources/org/pkl/core/errorMessages.properties b/pkl-core/src/main/resources/org/pkl/core/errorMessages.properties index 7ad9603aa..a8f365ad5 100644 --- a/pkl-core/src/main/resources/org/pkl/core/errorMessages.properties +++ b/pkl-core/src/main/resources/org/pkl/core/errorMessages.properties @@ -861,6 +861,9 @@ Package URIs must have an authority component.\n\ \n\ For example, `example.com` in URI `project://example.com/my/package@1.0.0`. +invalidRelativePathInPackageUri=\ +Package URIs cannot contain `..` segments in the URI's "path" component. + unexpectedChecksumInPackageUri=\ Did not expect to find a checksum component in this package URI. diff --git a/pkl-core/src/test/files/LanguageSnippetTests/input/packages/badImport12.error.pkl b/pkl-core/src/test/files/LanguageSnippetTests/input/packages/badImport12.error.pkl new file mode 100644 index 000000000..3420480cb --- /dev/null +++ b/pkl-core/src/test/files/LanguageSnippetTests/input/packages/badImport12.error.pkl @@ -0,0 +1,2 @@ +// relative path in package URI +res = import("package://localhost:0/%2e%2e/birds@0.5.0#/catalog/Bird.pkl") diff --git a/pkl-core/src/test/files/LanguageSnippetTests/output/packages/badImport12.err b/pkl-core/src/test/files/LanguageSnippetTests/output/packages/badImport12.err new file mode 100644 index 000000000..2cea3c34f --- /dev/null +++ b/pkl-core/src/test/files/LanguageSnippetTests/output/packages/badImport12.err @@ -0,0 +1,16 @@ +–– Pkl Error –– +Module URI `package://localhost:0/%2e%2e/birds@0.5.0#/catalog/Bird.pkl` has invalid syntax. + +x | res = import("package://localhost:0/%2e%2e/birds@0.5.0#/catalog/Bird.pkl") + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +at badImport12.error#res (file:///$snippetsDir/input/packages/badImport12.error.pkl) + +Package URIs cannot contain `..` segments in the URI's "path" component. + +xxx | renderer.renderDocument(value) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +at pkl.base#Module.output.text (pkl:base) + +xxx | if (renderer is BytesRenderer) renderer.renderDocument(value) else text.encodeToBytes("UTF-8") + ^^^^ +at pkl.base#Module.output.bytes (pkl:base) diff --git a/pkl-core/src/test/kotlin/org/pkl/core/packages/PackageUriTest.kt b/pkl-core/src/test/kotlin/org/pkl/core/packages/PackageUriTest.kt new file mode 100644 index 000000000..6abcb48f6 --- /dev/null +++ b/pkl-core/src/test/kotlin/org/pkl/core/packages/PackageUriTest.kt @@ -0,0 +1,55 @@ +/* + * Copyright © 2024-2026 Apple Inc. and the Pkl project authors. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.pkl.core.packages + +import java.net.URISyntaxException +import org.assertj.core.api.Assertions.assertThat +import org.assertj.core.api.Assertions.assertThatCode +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.assertThrows + +class PackageUriTest { + @Test + fun `rejects percent-encoded dot-dot path segments`() { + val err = + assertThrows { + PackageUri("package://attacker.com/%2e%2e/legit.example.com/legit@1.2.3") + } + assertThat(err).hasMessageContaining("..") + } + + @Test + fun `rejects literal dot-dot path segments`() { + assertThrows { PackageUri("package://attacker.com/../legit@1.2.3") } + } + + @Test + fun `rejects trailing dot-dot segment`() { + assertThrows { PackageUri("package://attacker.com/foo@1.2.3/%2e%2e") } + } + + @Test + fun `accepts a valid package URI`() { + assertThatCode { PackageUri("package://example.com/my/package@1.0.0") } + .doesNotThrowAnyException() + } + + @Test + fun `does not reject path segments that merely contain dots`() { + assertThatCode { PackageUri("package://example.com/my..pkg/..foo/bar..@1.0.0") } + .doesNotThrowAnyException() + } +}