Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion src/main/java/land/oras/OCILayout.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
8 changes: 7 additions & 1 deletion src/main/java/land/oras/Registry.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
60 changes: 60 additions & 0 deletions src/test/java/land/oras/OCILayoutTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
}
74 changes: 74 additions & 0 deletions src/test/java/land/oras/RegistryWireMockTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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");
}
}
Loading