Normalize and validate path from org.opencontainers.image.title (#703)

Signed-off-by: Valentin Delaye <jonesbusy@users.noreply.github.com>

Author: jonesbusy

src/main/java/land/oras/OCILayout.java

@@ -166,7 +166,13 @@ public void pullArtifact(LayoutRef ref, Path path, boolean overwrite) {
 
         // Copy the blob to the target path
         try {
-            Files.copy(blobPath, path.resolve(layer.getAnnotations().get(Const.ANNOTATION_TITLE)));
+            Path targetPath = path.resolve(layer.getAnnotations().get(Const.ANNOTATION_TITLE))
+                    .normalize();
+            if (!targetPath.startsWith(path.normalize())) {
+                throw new OrasException("Refusing to pull layer: title annotation is not withing folder '%s'"
+                        .formatted(layer.getAnnotations().get(Const.ANNOTATION_TITLE)));
+            }
+            Files.copy(blobPath, targetPath);
         } catch (IOException e) {
             throw new OrasException("Failed to copy blob", e);
         }

src/main/java/land/oras/Registry.java

@@ -1109,7 +1109,13 @@ private void pullLayer(ContainerRef ref, Layer layer, Path path, boolean overwri
                 ArchiveUtils.untar(Files.newInputStream(tempArchive.getPath()), path);
 
             } else {
-                Path targetPath = path.resolve(layer.getAnnotations().get(Const.ANNOTATION_TITLE));
+                Path targetPath = path.resolve(layer.getAnnotations().get(Const.ANNOTATION_TITLE))
+                        .normalize();
+                if (!targetPath.startsWith(path.normalize())) {
+                    throw new OrasException(
+                            "Refusing to pull layer: path is not withing folder in title annotation '%s'"
+                                    .formatted(layer.getAnnotations().get(Const.ANNOTATION_TITLE)));
+                }
                 if (Files.exists(targetPath) && !overwrite) {
                     LOG.info("File already exists: {}", targetPath);
                     return;

src/test/java/land/oras/OCILayoutTest.java

@@ -1149,4 +1149,64 @@ private void assertBlobContent(Path ociLayoutPath, String digest, String content
                         .resolve(SupportedAlgorithm.getDigest(digest))),
                 "Expect blob content to match");
     }
+
+    @Test
+    void pullArtifactShouldRejectInvalidTitleAnnotation() throws IOException {
+
+        // Create a valid OCI layout via the normal push API
+        Path ociLayoutPath = layoutPath.resolve("traversal-test");
+        Path artifactFile = blobDir.resolve("safe.txt");
+        Files.writeString(artifactFile, "safe content");
+
+        LayoutRef layoutRef = LayoutRef.parse("%s:latest".formatted(ociLayoutPath.toString()));
+        OCILayout ociLayout =
+                OCILayout.Builder.builder().defaults(ociLayoutPath).build();
+        ociLayout.pushArtifact(
+                layoutRef, ArtifactType.from("foo/bar"), Annotations.empty(), LocalPath.of(artifactFile, "text/plain"));
+
+        // 2. Read the manifest stored on disk and replace the title annotation with a traversal path
+        Index index = Index.fromPath(ociLayoutPath.resolve(Const.OCI_LAYOUT_INDEX));
+        String originalManifestDigest = index.getManifests().get(0).getDigest(); // e.g. sha256:abc...
+        Path manifestBlobPath = ociLayoutPath
+                .resolve("blobs")
+                .resolve("sha256")
+                .resolve(SupportedAlgorithm.getDigest(originalManifestDigest));
+
+        String originalManifestJson = Files.readString(manifestBlobPath);
+
+        // Invalid path
+        String tamperedManifestJson = originalManifestJson.replace("\"safe.txt\"", "\"../traversed-file.txt\"");
+
+        // Write tampered manifest as a new blob and update index.json
+        byte[] tamperedBytes = tamperedManifestJson.getBytes(StandardCharsets.UTF_8);
+        String tamperedDigest = SupportedAlgorithm.SHA256.digest(tamperedBytes);
+        Path tamperedBlobPath =
+                ociLayoutPath.resolve("blobs").resolve("sha256").resolve(SupportedAlgorithm.getDigest(tamperedDigest));
+        Files.write(tamperedBlobPath, tamperedBytes);
+
+        // Rewrite index.json to reference the tampered manifest digest
+        String updatedIndexJson = Files.readString(ociLayoutPath.resolve(Const.OCI_LAYOUT_INDEX))
+                .replace(
+                        SupportedAlgorithm.getDigest(originalManifestDigest),
+                        SupportedAlgorithm.getDigest(tamperedDigest));
+        Files.writeString(ociLayoutPath.resolve(Const.OCI_LAYOUT_INDEX), updatedIndexJson);
+
+        // Attempt to pull — must be rejected because the title escapes the output directory
+        Path outputDir = extractDir.resolve("traversal-output");
+        Files.createDirectories(outputDir);
+
+        OCILayout tampered = OCILayout.Builder.builder().defaults(ociLayoutPath).build();
+        OrasException exception = assertThrows(
+                OrasException.class,
+                () -> tampered.pullArtifact(layoutRef, outputDir, true),
+                "Expected OrasException for title annotation");
+        assertTrue(
+                exception.getMessage().contains("is not withing folder"),
+                "Exception message should mention is not withing folder but was: " + exception.getMessage());
+
+        // 5. The file must NOT have been written outside the output directory
+        assertFalse(
+                Files.exists(outputDir.getParent().resolve("traversed-file.txt")),
+                "Blob must not be written outside the output directory");
+    }
 }

src/test/java/land/oras/RegistryWireMockTest.java

@@ -23,8 +23,10 @@
 import static com.github.tomakehurst.wiremock.client.WireMock.*;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import com.github.tomakehurst.wiremock.WireMockServer;
 import com.github.tomakehurst.wiremock.client.WireMock;
@@ -1072,4 +1074,76 @@ private byte[] tokenScenario(
                 ContainerRef.parse("localhost:%d/library/%s".formatted(wmRuntimeInfo.getHttpPort(), registryName));
         return registry.getBlob(containerRef.withDigest(digest));
     }
+
+    @Test
+    void pullArtifactShouldRejectInvalidTitleAnnotation(WireMockRuntimeInfo wmRuntimeInfo) throws Exception {
+        WireMock wireMock = wmRuntimeInfo.getWireMock();
+        String registryUrl = wmRuntimeInfo.getHttpBaseUrl().replace("http://", "");
+
+        // Craft a blob and build a manifest whose layer title contains a invalid sequence
+        byte[] blobContent = "malicious content".getBytes(java.nio.charset.StandardCharsets.UTF_8);
+        String blobDigest = SupportedAlgorithm.SHA256.digest(blobContent);
+
+        Layer maliciousLayer = Layer.fromDigest(blobDigest, blobContent.length)
+                .withAnnotations(Map.of(Const.ANNOTATION_TITLE, "../traversed-file.txt"));
+
+        Manifest manifest = Manifest.empty().withLayers(List.of(maliciousLayer));
+        String manifestJson = JsonUtils.toJson(manifest);
+        String manifestDigest =
+                SupportedAlgorithm.SHA256.digest(manifestJson.getBytes(java.nio.charset.StandardCharsets.UTF_8));
+
+        // Stub HEAD manifest
+        wireMock.register(head(urlEqualTo("/v2/library/malicious-artifact/manifests/latest"))
+                .willReturn(aResponse()
+                        .withStatus(200)
+                        .withHeader(Const.CONTENT_TYPE_HEADER, Const.DEFAULT_MANIFEST_MEDIA_TYPE)
+                        .withHeader(Const.DOCKER_CONTENT_DIGEST_HEADER, manifestDigest)));
+
+        // Stub GET manifest
+        wireMock.register(get(urlEqualTo("/v2/library/malicious-artifact/manifests/latest"))
+                .willReturn(aResponse()
+                        .withStatus(200)
+                        .withHeader(Const.CONTENT_TYPE_HEADER, Const.DEFAULT_MANIFEST_MEDIA_TYPE)
+                        .withHeader(Const.DOCKER_CONTENT_DIGEST_HEADER, manifestDigest)
+                        .withBody(manifestJson)));
+
+        // Stub GET blob
+        wireMock.register(get(urlEqualTo("/v2/library/malicious-artifact/blobs/%s".formatted(blobDigest)))
+                .willReturn(aResponse()
+                        .withStatus(200)
+                        .withBody(blobContent)
+                        .withHeader(Const.DOCKER_CONTENT_DIGEST_HEADER, blobDigest)));
+
+        Registry registry = Registry.Builder.builder()
+                .withAuthProvider(authProvider)
+                .withInsecure(true)
+                .build();
+
+        ContainerRef containerRef = ContainerRef.parse("%s/library/malicious-artifact:latest".formatted(registryUrl));
+
+        Path outputDir = configDir.resolve("pull-output");
+        Files.createDirectories(outputDir);
+
+        // Check exception
+        Throwable cause = assertThrows(
+                Exception.class,
+                () -> registry.pullArtifact(containerRef, outputDir, true),
+                "Expected an exception for title annotation");
+        while (cause.getCause() != null && !(cause instanceof OrasException)) {
+            cause = cause.getCause();
+        }
+        assertInstanceOf(
+                OrasException.class,
+                cause,
+                "Root cause should be OrasException but was: "
+                        + cause.getClass().getName());
+        assertTrue(
+                cause.getMessage().contains("is not withing folder"),
+                "Exception message should mention is not withing folder but was: " + cause.getMessage());
+
+        // The file must NOT have been written outside the output directory
+        assertFalse(
+                Files.exists(outputDir.getParent().resolve("traversed-file.txt")),
+                "Blob must not be written outside the output directory");
+    }
 }