From 0046a442f9ac5ae186359df575e68fab17d01646 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Wed, 22 Apr 2026 17:39:41 -0700 Subject: [PATCH] Protect against corrupt buildpack archives Update zip and tar handling in buildpack code to ensure that archive entries cannot be written outside of the expected destination. Although we consider buildpacks to be trusted, this update will help protect against corrupt archives. Closes gh-50141 --- .../platform/build/ImageBuildpack.java | 5 ++++ .../platform/build/TarGzipBuildpack.java | 25 +++++++++------- .../platform/io/ZipFileTarArchive.java | 7 ++++- .../platform/build/ImageBuildpackTests.java | 30 +++++++++++++++---- .../platform/build/TarGzipBuildpackTests.java | 11 +++++++ .../buildpack/platform/build/TestTarGzip.java | 24 +++++++++------ .../platform/io/ZipFileTarArchiveTests.java | 22 ++++++++++---- 7 files changed, 93 insertions(+), 31 deletions(-) diff --git a/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/build/ImageBuildpack.java b/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/build/ImageBuildpack.java index 1714ee0994a..2d3620b6340 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/build/ImageBuildpack.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/build/ImageBuildpack.java @@ -37,6 +37,7 @@ import org.springframework.boot.buildpack.platform.docker.type.Layer; import org.springframework.boot.buildpack.platform.docker.type.LayerId; import org.springframework.boot.buildpack.platform.io.IOConsumer; import org.springframework.boot.buildpack.platform.io.TarArchive; +import org.springframework.util.Assert; import org.springframework.util.StreamUtils; /** @@ -132,6 +133,10 @@ final class ImageBuildpack implements Buildpack { out.setLongFileMode(TarArchiveOutputStream.LONGFILE_POSIX); TarArchiveEntry entry = in.getNextEntry(); while (entry != null) { + String entryName = entry.getName(); + Path entryPath = Path.of(entryName); + Assert.state(entryPath.toAbsolutePath().equals(entryPath.toAbsolutePath().normalize()), + () -> "Malformed zip entry name '%s'".formatted(entryName)); out.putArchiveEntry(entry); StreamUtils.copy(in, out); out.closeArchiveEntry(); diff --git a/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/build/TarGzipBuildpack.java b/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/build/TarGzipBuildpack.java index 23702dd9321..efd131bf221 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/build/TarGzipBuildpack.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/build/TarGzipBuildpack.java @@ -30,6 +30,7 @@ import org.apache.commons.compress.compressors.gzip.GzipCompressorInputStream; import org.springframework.boot.buildpack.platform.docker.type.Layer; import org.springframework.boot.buildpack.platform.io.IOConsumer; +import org.springframework.util.Assert; import org.springframework.util.StreamUtils; /** @@ -86,19 +87,23 @@ final class TarGzipBuildpack implements Buildpack { private void copyAndRebaseEntries(OutputStream outputStream) throws IOException { String id = this.coordinates.getSanitizedId(); Path basePath = Paths.get("/cnb/buildpacks/", id, this.coordinates.getVersion()); - try (TarArchiveInputStream tar = new TarArchiveInputStream( + try (TarArchiveInputStream tarInputStream = new TarArchiveInputStream( new GzipCompressorInputStream(Files.newInputStream(this.path))); - TarArchiveOutputStream output = new TarArchiveOutputStream(outputStream)) { - writeBasePathEntries(output, basePath); - TarArchiveEntry entry = tar.getNextEntry(); + TarArchiveOutputStream tarOutputStream = new TarArchiveOutputStream(outputStream)) { + writeBasePathEntries(tarOutputStream, basePath); + TarArchiveEntry entry = tarInputStream.getNextEntry(); while (entry != null) { - entry.setName(basePath + "/" + entry.getName()); - output.putArchiveEntry(entry); - StreamUtils.copy(tar, output); - output.closeArchiveEntry(); - entry = tar.getNextEntry(); + String entryName = entry.getName(); + Path entryPath = basePath.resolve(entryName); + Assert.state(entryPath.toAbsolutePath().normalize().startsWith(basePath.toAbsolutePath()), + () -> "Entry '%s' cannot be written outside of '%s'".formatted(entryName, basePath)); + entry.setName(basePath + "/" + entryName); + tarOutputStream.putArchiveEntry(entry); + StreamUtils.copy(tarInputStream, tarOutputStream); + tarOutputStream.closeArchiveEntry(); + entry = tarInputStream.getNextEntry(); } - output.finish(); + tarOutputStream.finish(); } } diff --git a/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/io/ZipFileTarArchive.java b/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/io/ZipFileTarArchive.java index 02d67cefa77..b82ef9f4189 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/io/ZipFileTarArchive.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/io/ZipFileTarArchive.java @@ -20,6 +20,7 @@ import java.io.File; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.nio.file.Path; import java.util.Enumeration; import org.apache.commons.compress.archivers.tar.TarArchiveEntry; @@ -94,7 +95,11 @@ public class ZipFileTarArchive implements TarArchive { private TarArchiveEntry convert(ZipArchiveEntry zipEntry) { byte linkFlag = (zipEntry.isDirectory()) ? TarConstants.LF_DIR : TarConstants.LF_NORMAL; - TarArchiveEntry tarEntry = new TarArchiveEntry(zipEntry.getName(), linkFlag, true); + String entryName = zipEntry.getName(); + Path entryPath = Path.of(entryName); + Assert.state(entryPath.toAbsolutePath().equals(entryPath.toAbsolutePath().normalize()), + () -> "Malformed zip entry name '%s'".formatted(entryName)); + TarArchiveEntry tarEntry = new TarArchiveEntry(entryName, linkFlag, true); tarEntry.setUserId(this.owner.getUid()); tarEntry.setGroupId(this.owner.getGid()); tarEntry.setModTime(NORMALIZED_MOD_TIME); diff --git a/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/build/ImageBuildpackTests.java b/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/build/ImageBuildpackTests.java index dadfc07d89c..f6fec636268 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/build/ImageBuildpackTests.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/build/ImageBuildpackTests.java @@ -177,18 +177,36 @@ class ImageBuildpackTests extends AbstractJsonTests { assertThat(buildpack).isNull(); } + @Test + void resolveWhenEntryWouldWriteOutsideOfDestinationThrowsException() throws Exception { + Image image = Image.of(getContent("buildpack-image.json")); + ImageReference imageReference = ImageReference.of("example/buildpack1:latest"); + BuildpackResolverContext resolverContext = mock(BuildpackResolverContext.class); + given(resolverContext.getBuildpackLayersMetadata()).willReturn(BuildpackLayersMetadata.fromJson("{}")); + given(resolverContext.fetchImage(eq(imageReference), eq(ImageType.BUILDPACK))).willReturn(image); + willAnswer((invocation) -> withMockLayers(invocation, "..")).given(resolverContext) + .exportImageLayers(eq(imageReference), any()); + BuildpackReference reference = BuildpackReference.of("example/buildpack1"); + assertThatIllegalStateException().isThrownBy(() -> ImageBuildpack.resolve(resolverContext, reference)) + .withMessage("Malformed zip entry name '../cnb/'"); + } + private Object withMockLayers(InvocationOnMock invocation) { + return withMockLayers(invocation, ""); + } + + private Object withMockLayers(InvocationOnMock invocation, String entryPrefix) { try { IOBiConsumer consumer = invocation.getArgument(1); File tarFile = File.createTempFile("create-builder-test-", null); try (TarArchiveOutputStream tarOut = new TarArchiveOutputStream(new FileOutputStream(tarFile))) { tarOut.setLongFileMode(TarArchiveOutputStream.LONGFILE_POSIX); - writeTarEntry(tarOut, "/cnb/"); - writeTarEntry(tarOut, "/cnb/buildpacks/"); - writeTarEntry(tarOut, "/cnb/buildpacks/example_buildpack/"); - writeTarEntry(tarOut, "/cnb/buildpacks/example_buildpack/0.0.1/"); - writeTarEntry(tarOut, "/cnb/buildpacks/example_buildpack/0.0.1/buildpack.toml"); - writeTarEntry(tarOut, "/cnb/buildpacks/example_buildpack/0.0.1/" + this.longFilePath); + writeTarEntry(tarOut, entryPrefix + "/cnb/"); + writeTarEntry(tarOut, entryPrefix + "/cnb/buildpacks/"); + writeTarEntry(tarOut, entryPrefix + "/cnb/buildpacks/example_buildpack/"); + writeTarEntry(tarOut, entryPrefix + "/cnb/buildpacks/example_buildpack/0.0.1/"); + writeTarEntry(tarOut, entryPrefix + "/cnb/buildpacks/example_buildpack/0.0.1/buildpack.toml"); + writeTarEntry(tarOut, entryPrefix + "/cnb/buildpacks/example_buildpack/0.0.1/" + this.longFilePath); tarOut.finish(); } try (FileInputStream tarFileStream = new FileInputStream(tarFile)) { diff --git a/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/build/TarGzipBuildpackTests.java b/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/build/TarGzipBuildpackTests.java index b50b7d3590d..4641d1cae1f 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/build/TarGzipBuildpackTests.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/build/TarGzipBuildpackTests.java @@ -25,6 +25,7 @@ import org.junit.jupiter.api.io.TempDir; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; +import static org.assertj.core.api.Assertions.assertThatIllegalStateException; import static org.mockito.Mockito.mock; /** @@ -91,4 +92,14 @@ class TarGzipBuildpackTests { assertThat(buildpack).isNull(); } + @Test + void resolveWillNotIApplyEntriesOutsideOfOutputLocation() throws Exception { + Path compressedArchive = this.testTarGzip + .createArchive((entryName) -> entryName.endsWith(".toml") ? entryName : "../" + entryName); + BuildpackReference reference = BuildpackReference.of(compressedArchive.toUri().toString()); + Buildpack buildpack = TarGzipBuildpack.resolve(this.resolverContext, reference); + assertThatIllegalStateException().isThrownBy(() -> buildpack.apply((layers) -> { + })).withMessage("Entry '../bin/' cannot be written outside of '/cnb/buildpacks/example_buildpack1/0.0.1'"); + } + } diff --git a/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/build/TestTarGzip.java b/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/build/TestTarGzip.java index c849f7d7124..3969f62b351 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/build/TestTarGzip.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/build/TestTarGzip.java @@ -26,6 +26,7 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.util.ArrayList; import java.util.List; +import java.util.function.UnaryOperator; import org.apache.commons.compress.archivers.tar.TarArchiveEntry; import org.apache.commons.compress.archivers.tar.TarArchiveInputStream; @@ -51,18 +52,22 @@ class TestTarGzip { } Path createArchive() throws Exception { - return createArchive(true); + return createArchive(true, UnaryOperator.identity()); + } + + Path createArchive(UnaryOperator entryNameProcessor) throws Exception { + return createArchive(true, entryNameProcessor); } Path createEmptyArchive() throws Exception { - return createArchive(false); + return createArchive(false, UnaryOperator.identity()); } - private Path createArchive(boolean addContent) throws Exception { + private Path createArchive(boolean addContent, UnaryOperator entryNameProcessor) throws Exception { Path path = Paths.get(this.buildpackDir.getAbsolutePath(), "buildpack.tar"); Path archive = Files.createFile(path); if (addContent) { - writeBuildpackContentToArchive(archive); + writeBuildpackContentToArchive(archive, entryNameProcessor); } return compressBuildpackArchive(archive); } @@ -74,7 +79,8 @@ class TestTarGzip { return tgzPath; } - private void writeBuildpackContentToArchive(Path archive) throws Exception { + private void writeBuildpackContentToArchive(Path archive, UnaryOperator entryNameProcessor) + throws Exception { StringBuilder buildpackToml = new StringBuilder(); buildpackToml.append("[buildpack]\n"); buildpackToml.append("id = \"example/buildpack1\"\n"); @@ -92,10 +98,10 @@ class TestTarGzip { echo "---> build" """; try (TarArchiveOutputStream tar = new TarArchiveOutputStream(Files.newOutputStream(archive))) { - writeEntry(tar, "buildpack.toml", buildpackToml.toString()); - writeEntry(tar, "bin/"); - writeEntry(tar, "bin/detect", detectScript); - writeEntry(tar, "bin/build", buildScript); + writeEntry(tar, entryNameProcessor.apply("buildpack.toml"), buildpackToml.toString()); + writeEntry(tar, entryNameProcessor.apply("bin/")); + writeEntry(tar, entryNameProcessor.apply("bin/detect"), detectScript); + writeEntry(tar, entryNameProcessor.apply("bin/build"), buildScript); tar.finish(); } } diff --git a/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/io/ZipFileTarArchiveTests.java b/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/io/ZipFileTarArchiveTests.java index 62add8c36a6..784d94c9b35 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/io/ZipFileTarArchiveTests.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/io/ZipFileTarArchiveTests.java @@ -31,6 +31,7 @@ import org.junit.jupiter.api.io.TempDir; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; +import static org.assertj.core.api.Assertions.assertThatIllegalStateException; /** * Tests for {@link ZipFileTarArchive}. @@ -52,7 +53,7 @@ class ZipFileTarArchiveTests { @Test void createWhenOwnerIsNullThrowsException() throws Exception { File file = new File(this.tempDir, "test.zip"); - writeTestZip(file); + writeTestZip(file, ""); assertThatIllegalArgumentException().isThrownBy(() -> new ZipFileTarArchive(file, null)) .withMessage("'owner' must not be null"); } @@ -61,7 +62,7 @@ class ZipFileTarArchiveTests { void writeToAdaptsContent() throws Exception { Owner owner = Owner.of(123, 456); File file = new File(this.tempDir, "test.zip"); - writeTestZip(file); + writeTestZip(file, ""); TarArchive tarArchive = TarArchive.fromZip(file, owner); ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); tarArchive.writeTo(outputStream); @@ -81,12 +82,23 @@ class ZipFileTarArchiveTests { } } - private void writeTestZip(File file) throws IOException { + @Test + void writeToDoesNotIncludeEntriesThatWouldBeWrittenOutsideOfDestination() throws Exception { + Owner owner = Owner.of(123, 456); + File file = new File(this.tempDir, "test.zip"); + writeTestZip(file, "../"); + TarArchive tarArchive = TarArchive.fromZip(file, owner); + ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); + assertThatIllegalStateException().isThrownBy(() -> tarArchive.writeTo(outputStream)) + .withMessage("Malformed zip entry name '../spring/'"); + } + + private void writeTestZip(File file, String prefix) throws IOException { try (ZipArchiveOutputStream zip = new ZipArchiveOutputStream(file)) { - ZipArchiveEntry dirEntry = new ZipArchiveEntry("spring/"); + ZipArchiveEntry dirEntry = new ZipArchiveEntry(prefix + "spring/"); zip.putArchiveEntry(dirEntry); zip.closeArchiveEntry(); - ZipArchiveEntry fileEntry = new ZipArchiveEntry("spring/boot"); + ZipArchiveEntry fileEntry = new ZipArchiveEntry(prefix + "spring/boot"); fileEntry.setUnixMode(0755); zip.putArchiveEntry(fileEntry); zip.write("test".getBytes(StandardCharsets.UTF_8));