Skip to content

Commit 3c7e861

Browse files
philwebbphilwebb
committed
Merge branch '4.0.x'
Closes gh-50165
2 parents 0277a4f + c38383b commit 3c7e861

7 files changed

Lines changed: 93 additions & 31 deletions

File tree

‎buildpack/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/build/ImageBuildpack.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,10 @@ private Path createLayerFile(TarArchive tarArchive) throws IOException {
141141
out.setLongFileMode(TarArchiveOutputStream.LONGFILE_POSIX);
142142
TarArchiveEntry entry = in.getNextEntry();
143143
while (entry != null) {
144+
String entryName = entry.getName();
145+
Path entryPath = Path.of(entryName);
146+
Assert.state(entryPath.toAbsolutePath().equals(entryPath.toAbsolutePath().normalize()),
147+
() -> "Malformed zip entry name '%s'".formatted(entryName));
144148
out.putArchiveEntry(entry);
145149
StreamUtils.copy(in, out);
146150
out.closeArchiveEntry();

‎buildpack/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/build/TarGzipBuildpack.java‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131

3232
import org.springframework.boot.buildpack.platform.docker.type.Layer;
3333
import org.springframework.boot.buildpack.platform.io.IOConsumer;
34+
import org.springframework.util.Assert;
3435
import org.springframework.util.StreamUtils;
3536

3637
/**
@@ -87,19 +88,23 @@ public void apply(IOConsumer<Layer> layers) throws IOException {
8788
private void copyAndRebaseEntries(OutputStream outputStream) throws IOException {
8889
String id = this.coordinates.getSanitizedId();
8990
Path basePath = Paths.get("/cnb/buildpacks/", id, this.coordinates.getVersion());
90-
try (TarArchiveInputStream tar = new TarArchiveInputStream(
91+
try (TarArchiveInputStream tarInputStream = new TarArchiveInputStream(
9192
new GzipCompressorInputStream(Files.newInputStream(this.path)));
92-
TarArchiveOutputStream output = new TarArchiveOutputStream(outputStream)) {
93-
writeBasePathEntries(output, basePath);
94-
TarArchiveEntry entry = tar.getNextEntry();
93+
TarArchiveOutputStream tarOutputStream = new TarArchiveOutputStream(outputStream)) {
94+
writeBasePathEntries(tarOutputStream, basePath);
95+
TarArchiveEntry entry = tarInputStream.getNextEntry();
9596
while (entry != null) {
96-
entry.setName(basePath + "/" + entry.getName());
97-
output.putArchiveEntry(entry);
98-
StreamUtils.copy(tar, output);
99-
output.closeArchiveEntry();
100-
entry = tar.getNextEntry();
97+
String entryName = entry.getName();
98+
Path entryPath = basePath.resolve(entryName);
99+
Assert.state(entryPath.toAbsolutePath().normalize().startsWith(basePath.toAbsolutePath()),
100+
() -> "Entry '%s' cannot be written outside of '%s'".formatted(entryName, basePath));
101+
entry.setName(basePath + "/" + entryName);
102+
tarOutputStream.putArchiveEntry(entry);
103+
StreamUtils.copy(tarInputStream, tarOutputStream);
104+
tarOutputStream.closeArchiveEntry();
105+
entry = tarInputStream.getNextEntry();
101106
}
102-
output.finish();
107+
tarOutputStream.finish();
103108
}
104109
}
105110

‎buildpack/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/io/ZipFileTarArchive.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import java.io.IOException;
2121
import java.io.InputStream;
2222
import java.io.OutputStream;
23+
import java.nio.file.Path;
2324
import java.util.Enumeration;
2425

2526
import org.apache.commons.compress.archivers.tar.TarArchiveEntry;
@@ -94,7 +95,11 @@ private void copy(ZipArchiveEntry zipEntry, InputStream zip, TarArchiveOutputStr
9495

9596
private TarArchiveEntry convert(ZipArchiveEntry zipEntry) {
9697
byte linkFlag = (zipEntry.isDirectory()) ? TarConstants.LF_DIR : TarConstants.LF_NORMAL;
97-
TarArchiveEntry tarEntry = new TarArchiveEntry(zipEntry.getName(), linkFlag, true);
98+
String entryName = zipEntry.getName();
99+
Path entryPath = Path.of(entryName);
100+
Assert.state(entryPath.toAbsolutePath().equals(entryPath.toAbsolutePath().normalize()),
101+
() -> "Malformed zip entry name '%s'".formatted(entryName));
102+
TarArchiveEntry tarEntry = new TarArchiveEntry(entryName, linkFlag, true);
98103
tarEntry.setUserId(this.owner.getUid());
99104
tarEntry.setGroupId(this.owner.getGid());
100105
tarEntry.setModTime(NORMALIZED_MOD_TIME);

‎buildpack/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/build/ImageBuildpackTests.java‎

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -183,18 +183,36 @@ void resolveWhenUnqualifiedReferenceWithInvalidImageReferenceReturnsNull() {
183183
assertThat(buildpack).isNull();
184184
}
185185

186+
@Test
187+
void resolveWhenEntryWouldWriteOutsideOfDestinationThrowsException() throws Exception {
188+
Image image = Image.of(getContent("buildpack-image.json"));
189+
ImageReference imageReference = ImageReference.of("example/buildpack1:latest");
190+
BuildpackResolverContext resolverContext = mock(BuildpackResolverContext.class);
191+
given(resolverContext.getBuildpackLayersMetadata()).willReturn(BuildpackLayersMetadata.fromJson("{}"));
192+
given(resolverContext.fetchImage(eq(imageReference), eq(ImageType.BUILDPACK))).willReturn(image);
193+
willAnswer((invocation) -> withMockLayers(invocation, "..")).given(resolverContext)
194+
.exportImageLayers(eq(imageReference), any());
195+
BuildpackReference reference = BuildpackReference.of("example/buildpack1");
196+
assertThatIllegalStateException().isThrownBy(() -> ImageBuildpack.resolve(resolverContext, reference))
197+
.withMessage("Malformed zip entry name '../cnb/'");
198+
}
199+
186200
private @Nullable Object withMockLayers(InvocationOnMock invocation) {
201+
return withMockLayers(invocation, "");
202+
}
203+
204+
private @Nullable Object withMockLayers(InvocationOnMock invocation, String entryPrefix) {
187205
try {
188206
IOBiConsumer<String, TarArchive> consumer = invocation.getArgument(1);
189207
File tarFile = File.createTempFile("create-builder-test-", null);
190208
try (TarArchiveOutputStream tarOut = new TarArchiveOutputStream(new FileOutputStream(tarFile))) {
191209
tarOut.setLongFileMode(TarArchiveOutputStream.LONGFILE_POSIX);
192-
writeTarEntry(tarOut, "/cnb/");
193-
writeTarEntry(tarOut, "/cnb/buildpacks/");
194-
writeTarEntry(tarOut, "/cnb/buildpacks/example_buildpack/");
195-
writeTarEntry(tarOut, "/cnb/buildpacks/example_buildpack/0.0.1/");
196-
writeTarEntry(tarOut, "/cnb/buildpacks/example_buildpack/0.0.1/buildpack.toml");
197-
writeTarEntry(tarOut, "/cnb/buildpacks/example_buildpack/0.0.1/" + this.longFilePath);
210+
writeTarEntry(tarOut, entryPrefix + "/cnb/");
211+
writeTarEntry(tarOut, entryPrefix + "/cnb/buildpacks/");
212+
writeTarEntry(tarOut, entryPrefix + "/cnb/buildpacks/example_buildpack/");
213+
writeTarEntry(tarOut, entryPrefix + "/cnb/buildpacks/example_buildpack/0.0.1/");
214+
writeTarEntry(tarOut, entryPrefix + "/cnb/buildpacks/example_buildpack/0.0.1/buildpack.toml");
215+
writeTarEntry(tarOut, entryPrefix + "/cnb/buildpacks/example_buildpack/0.0.1/" + this.longFilePath);
198216
tarOut.finish();
199217
}
200218
try (FileInputStream tarFileStream = new FileInputStream(tarFile)) {

‎buildpack/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/build/TarGzipBuildpackTests.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525

2626
import static org.assertj.core.api.Assertions.assertThat;
2727
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
28+
import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
2829
import static org.mockito.Mockito.mock;
2930

3031
/**
@@ -91,4 +92,15 @@ void resolveWhenArchiveThatDoesNotExistReturnsNull() {
9192
assertThat(buildpack).isNull();
9293
}
9394

95+
@Test
96+
void resolveWillNotIApplyEntriesOutsideOfOutputLocation() throws Exception {
97+
Path compressedArchive = this.testTarGzip
98+
.createArchive((entryName) -> entryName.endsWith(".toml") ? entryName : "../" + entryName);
99+
BuildpackReference reference = BuildpackReference.of(compressedArchive.toUri().toString());
100+
Buildpack buildpack = TarGzipBuildpack.resolve(this.resolverContext, reference);
101+
assertThat(buildpack).as("Buildpack %s resolved from reference %s", buildpack, reference).isNotNull();
102+
assertThatIllegalStateException().isThrownBy(() -> buildpack.apply((layers) -> {
103+
})).withMessage("Entry '../bin/' cannot be written outside of '/cnb/buildpacks/example_buildpack1/0.0.1'");
104+
}
105+
94106
}

‎buildpack/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/build/TestTarGzip.java‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import java.nio.file.Paths;
2727
import java.util.ArrayList;
2828
import java.util.List;
29+
import java.util.function.UnaryOperator;
2930

3031
import org.apache.commons.compress.archivers.tar.TarArchiveEntry;
3132
import org.apache.commons.compress.archivers.tar.TarArchiveInputStream;
@@ -51,18 +52,22 @@ class TestTarGzip {
5152
}
5253

5354
Path createArchive() throws Exception {
54-
return createArchive(true);
55+
return createArchive(true, UnaryOperator.identity());
56+
}
57+
58+
Path createArchive(UnaryOperator<String> entryNameProcessor) throws Exception {
59+
return createArchive(true, entryNameProcessor);
5560
}
5661

5762
Path createEmptyArchive() throws Exception {
58-
return createArchive(false);
63+
return createArchive(false, UnaryOperator.identity());
5964
}
6065

61-
private Path createArchive(boolean addContent) throws Exception {
66+
private Path createArchive(boolean addContent, UnaryOperator<String> entryNameProcessor) throws Exception {
6267
Path path = Paths.get(this.buildpackDir.getAbsolutePath(), "buildpack.tar");
6368
Path archive = Files.createFile(path);
6469
if (addContent) {
65-
writeBuildpackContentToArchive(archive);
70+
writeBuildpackContentToArchive(archive, entryNameProcessor);
6671
}
6772
return compressBuildpackArchive(archive);
6873
}
@@ -74,7 +79,8 @@ private Path compressBuildpackArchive(Path archive) throws Exception {
7479
return tgzPath;
7580
}
7681

77-
private void writeBuildpackContentToArchive(Path archive) throws Exception {
82+
private void writeBuildpackContentToArchive(Path archive, UnaryOperator<String> entryNameProcessor)
83+
throws Exception {
7884
StringBuilder buildpackToml = new StringBuilder();
7985
buildpackToml.append("[buildpack]\n");
8086
buildpackToml.append("id = \"example/buildpack1\"\n");
@@ -92,10 +98,10 @@ private void writeBuildpackContentToArchive(Path archive) throws Exception {
9298
echo "---> build"
9399
""";
94100
try (TarArchiveOutputStream tar = new TarArchiveOutputStream(Files.newOutputStream(archive))) {
95-
writeEntry(tar, "buildpack.toml", buildpackToml.toString());
96-
writeEntry(tar, "bin/");
97-
writeEntry(tar, "bin/detect", detectScript);
98-
writeEntry(tar, "bin/build", buildScript);
101+
writeEntry(tar, entryNameProcessor.apply("buildpack.toml"), buildpackToml.toString());
102+
writeEntry(tar, entryNameProcessor.apply("bin/"));
103+
writeEntry(tar, entryNameProcessor.apply("bin/detect"), detectScript);
104+
writeEntry(tar, entryNameProcessor.apply("bin/build"), buildScript);
99105
tar.finish();
100106
}
101107
}

‎buildpack/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/io/ZipFileTarArchiveTests.java‎

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131

3232
import static org.assertj.core.api.Assertions.assertThat;
3333
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
34+
import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
3435

3536
/**
3637
* Tests for {@link ZipFileTarArchive}.
@@ -55,7 +56,7 @@ void createWhenZipIsNullThrowsException() {
5556
@SuppressWarnings("NullAway") // Test null check
5657
void createWhenOwnerIsNullThrowsException() throws Exception {
5758
File file = new File(this.tempDir, "test.zip");
58-
writeTestZip(file);
59+
writeTestZip(file, "");
5960
assertThatIllegalArgumentException().isThrownBy(() -> new ZipFileTarArchive(file, null))
6061
.withMessage("'owner' must not be null");
6162
}
@@ -64,7 +65,7 @@ void createWhenOwnerIsNullThrowsException() throws Exception {
6465
void writeToAdaptsContent() throws Exception {
6566
Owner owner = Owner.of(123, 456);
6667
File file = new File(this.tempDir, "test.zip");
67-
writeTestZip(file);
68+
writeTestZip(file, "");
6869
TarArchive tarArchive = TarArchive.fromZip(file, owner);
6970
ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
7071
tarArchive.writeTo(outputStream);
@@ -84,12 +85,23 @@ void writeToAdaptsContent() throws Exception {
8485
}
8586
}
8687

87-
private void writeTestZip(File file) throws IOException {
88+
@Test
89+
void writeToDoesNotIncludeEntriesThatWouldBeWrittenOutsideOfDestination() throws Exception {
90+
Owner owner = Owner.of(123, 456);
91+
File file = new File(this.tempDir, "test.zip");
92+
writeTestZip(file, "../");
93+
TarArchive tarArchive = TarArchive.fromZip(file, owner);
94+
ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
95+
assertThatIllegalStateException().isThrownBy(() -> tarArchive.writeTo(outputStream))
96+
.withMessage("Malformed zip entry name '../spring/'");
97+
}
98+
99+
private void writeTestZip(File file, String prefix) throws IOException {
88100
try (ZipArchiveOutputStream zip = new ZipArchiveOutputStream(file)) {
89-
ZipArchiveEntry dirEntry = new ZipArchiveEntry("spring/");
101+
ZipArchiveEntry dirEntry = new ZipArchiveEntry(prefix + "spring/");
90102
zip.putArchiveEntry(dirEntry);
91103
zip.closeArchiveEntry();
92-
ZipArchiveEntry fileEntry = new ZipArchiveEntry("spring/boot");
104+
ZipArchiveEntry fileEntry = new ZipArchiveEntry(prefix + "spring/boot");
93105
fileEntry.setUnixMode(0755);
94106
zip.putArchiveEntry(fileEntry);
95107
zip.write("test".getBytes(StandardCharsets.UTF_8));

0 commit comments

Comments
 (0)