Skip to content

Commit 083fe12

Browse files
authored
Merge commit from fork
untar and unzip created symlink entries directly from the archive without checking that the link target resolved inside the extraction directory. A tar that planted a symlink whose target pointed above the output dir, followed by a regular-file entry whose path traversed through that symlink, allowed Files.newOutputStream to write outside the caller's chosen target. This commit adds an ensureSafeSymlinkTarget helper that mirrors the existing ensureSafeEntry containment check, plus switches the regular-file write path to open with CREATE_NEW and LinkOption.NOFOLLOW_LINKS so a pre-existing symlink under the extraction path cannot be followed. The shouldRejectSymlinkEscapingTargetOnUntar regression test in ArchiveUtilsTest builds the malicious tar inline and asserts no file lands outside target. The existing tar/zip round-trip tests are updated to construct the source fixture's symlink with a relative target (the safe form), matching what well-formed OCI layers carry. Signed-off-by: tonghuaroot <tonghuaroot@gmail.com>
1 parent 8e39dd5 commit 083fe12

2 files changed

Lines changed: 80 additions & 6 deletions

File tree

src/main/java/land/oras/utils/ArchiveUtils.java

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,10 @@
2727
import java.io.OutputStream;
2828
import java.nio.charset.StandardCharsets;
2929
import java.nio.file.Files;
30+
import java.nio.file.LinkOption;
3031
import java.nio.file.Path;
3132
import java.nio.file.Paths;
33+
import java.nio.file.StandardOpenOption;
3234
import java.nio.file.attribute.BasicFileAttributes;
3335
import java.nio.file.attribute.PosixFilePermission;
3436
import java.util.EnumSet;
@@ -267,6 +269,25 @@ static void ensureSafeEntry(ArchiveEntry entry, Path target) throws IOException
267269
}
268270
}
269271

272+
/**
273+
* Ensure that a symlink entry's link target resolves inside the extraction directory.
274+
* Stops an archive from planting a symlink that points outside the target tree, which
275+
* would otherwise let subsequent regular-file entries write outside the target dir.
276+
* @param outputPath The on-disk path where the symlink will be created
277+
* @param linkTarget The raw link target string from the archive entry
278+
* @param target The extraction root
279+
* @throws IOException if the link target escapes the extraction root
280+
*/
281+
static void ensureSafeSymlinkTarget(Path outputPath, Path linkTarget, Path target) throws IOException {
282+
Path normalizedTarget = target.toAbsolutePath().normalize();
283+
Path resolved =
284+
(linkTarget.isAbsolute() ? linkTarget : outputPath.getParent().resolve(linkTarget)).normalize();
285+
if (!resolved.startsWith(normalizedTarget)) {
286+
throw new IOException(
287+
"Refusing to create symlink that escapes target dir: " + outputPath + " -> " + linkTarget);
288+
}
289+
}
290+
270291
/**
271292
* Extract a tar file to a target directory
272293
* @param path The tar file
@@ -376,11 +397,17 @@ static void unzip(InputStream fis, Path target) {
376397
String linkStr = asiField != null
377398
? asiField.getLinkedFile()
378399
: new String(zais.readAllBytes(), StandardCharsets.UTF_8);
379-
createSymbolicLink(outputPath, Paths.get(linkStr));
400+
Path linkPath = Paths.get(linkStr);
401+
ensureSafeSymlinkTarget(outputPath, linkPath, target);
402+
createSymbolicLink(outputPath, linkPath);
380403
} else {
381404
LOG.debug("Extracting file: {}", entry.getName());
382405
Files.createDirectories(outputPath.getParent());
383-
try (OutputStream out = Files.newOutputStream(outputPath)) {
406+
try (OutputStream out = Files.newOutputStream(
407+
outputPath,
408+
StandardOpenOption.CREATE_NEW,
409+
StandardOpenOption.WRITE,
410+
LinkOption.NOFOLLOW_LINKS)) {
384411
zais.transferTo(out);
385412
}
386413
}
@@ -425,9 +452,15 @@ public static void untar(InputStream fis, Path target) {
425452

426453
// Restore file permissions (optional, based on your need)
427454
if (entry.isSymbolicLink()) {
428-
createSymbolicLink(outputPath, Paths.get(entry.getLinkName()));
455+
Path linkPath = Paths.get(entry.getLinkName());
456+
ensureSafeSymlinkTarget(outputPath, linkPath, target);
457+
createSymbolicLink(outputPath, linkPath);
429458
} else {
430-
try (OutputStream out = Files.newOutputStream(outputPath)) {
459+
try (OutputStream out = Files.newOutputStream(
460+
outputPath,
461+
StandardOpenOption.CREATE_NEW,
462+
StandardOpenOption.WRITE,
463+
LinkOption.NOFOLLOW_LINKS)) {
431464
tais.transferTo(out);
432465
}
433466
if (OsUtils.isPosixFileSystemSupported()) {

src/test/java/land/oras/utils/ArchiveUtilsTest.java

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import land.oras.LocalPath;
3434
import land.oras.exception.OrasException;
3535
import org.apache.commons.compress.archivers.tar.TarArchiveEntry;
36+
import org.apache.commons.compress.archivers.tar.TarArchiveOutputStream;
3637
import org.junit.jupiter.api.BeforeAll;
3738
import org.junit.jupiter.api.Disabled;
3839
import org.junit.jupiter.api.Test;
@@ -92,10 +93,11 @@ static void beforeAll() throws Exception {
9293
Files.writeString(file2, "file2");
9394
Files.writeString(file4, "file4");
9495

95-
// Create one symlink file3 -> file1
96+
// Create one symlink file3 -> file1 (use a relative target so the extracted
97+
// symlink stays inside the extraction directory; absolute targets would escape)
9698
if (OsUtils.isPosixFileSystemSupported()) {
9799
Path file3 = dir1.resolve("file3");
98-
Files.createSymbolicLink(file3, file1);
100+
Files.createSymbolicLink(file3, Paths.get("file1"));
99101

100102
// Add 777 permission to file2
101103
Files.setPosixFilePermissions(
@@ -328,4 +330,43 @@ void shouldCreateTarZstdAndExtractIt() throws Exception {
328330
Path temp = ArchiveUtils.uncompressuntar(compressedArchive, directory.getMediaType());
329331
assertTrue(Files.exists(temp), "Temp should exist");
330332
}
333+
334+
/**
335+
* Verify that untar rejects a tar that plants a symlink whose target escapes
336+
* the extraction directory, even when the entry name itself stays under target.
337+
* A second regular-file entry whose name traverses through the planted symlink
338+
* would otherwise resolve to a path outside target.
339+
*/
340+
@Test
341+
void shouldRejectSymlinkEscapingTargetOnUntar(@TempDir Path tmp) throws IOException {
342+
if (!OsUtils.isPosixFileSystemSupported()) {
343+
return;
344+
}
345+
Path target = tmp.resolve("safe-output");
346+
Files.createDirectories(target);
347+
Path escapeFile = tmp.resolve("ESCAPED.txt");
348+
Files.deleteIfExists(escapeFile);
349+
350+
Path mtar = tmp.resolve("malicious.tar");
351+
try (TarArchiveOutputStream tout = new TarArchiveOutputStream(Files.newOutputStream(mtar))) {
352+
tout.setLongFileMode(TarArchiveOutputStream.LONGFILE_POSIX);
353+
354+
TarArchiveEntry symlinkEntry = new TarArchiveEntry("evil-link", TarArchiveEntry.LF_SYMLINK);
355+
symlinkEntry.setLinkName(tmp.toAbsolutePath().toString());
356+
symlinkEntry.setMode(0777);
357+
tout.putArchiveEntry(symlinkEntry);
358+
tout.closeArchiveEntry();
359+
360+
byte[] data = "should not land outside target\n".getBytes();
361+
TarArchiveEntry fileEntry = new TarArchiveEntry("evil-link/ESCAPED.txt");
362+
fileEntry.setSize(data.length);
363+
fileEntry.setMode(0644);
364+
tout.putArchiveEntry(fileEntry);
365+
tout.write(data);
366+
tout.closeArchiveEntry();
367+
}
368+
369+
assertThrows(OrasException.class, () -> ArchiveUtils.untar(mtar, target));
370+
assertFalse(Files.exists(escapeFile), "Symlink-target escape must not create a file outside target");
371+
}
331372
}

0 commit comments

Comments
 (0)