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));