Skip to content

Commit dc410db

Browse files
fix: unpack prod bundle on Windows mapped network drives (CP: 24.10) (#24564) (CP: 24.9) (#24565)
This PR cherry-picks changes from the original PR #24564 to branch 24.9. --- #### Original PR description > The Zip Slip guard in `CompressUtil.newFile` canonicalized the target dir and the not-yet-created entry separately; on mapped/`subst` drives `getCanonicalPath()` returns inconsistent forms, so legitimate entries were rejected. > > Canonicalize the destination once and resolve entries lexically, checking containment with `Path.startsWith`. > > Fixes #24558 > > Cherry-pick of #24560 > Co-authored-by: Marco Collovati <marco@vaadin.com>
1 parent 8e7040a commit dc410db

2 files changed

Lines changed: 91 additions & 7 deletions

File tree

flow-server/src/main/java/com/vaadin/flow/server/frontend/CompressUtil.java

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import java.io.IOException;
2323
import java.io.InputStream;
2424
import java.nio.charset.StandardCharsets;
25+
import java.nio.file.Path;
2526
import java.util.zip.ZipEntry;
2627
import java.util.zip.ZipException;
2728
import java.util.zip.ZipFile;
@@ -145,17 +146,21 @@ public static void uncompressFile(File zip, File targetDirectory) {
145146

146147
private static File newFile(File destinationDir, ZipEntry zipEntry)
147148
throws IOException {
148-
File destFile = new File(destinationDir, zipEntry.getName());
149-
150-
String destDirPath = destinationDir.getCanonicalPath();
151-
String destFilePath = destFile.getCanonicalPath();
152-
153-
if (!destFilePath.startsWith(destDirPath + File.separator)) {
149+
// Canonicalize the destination directory once. Resolving each entry
150+
// lexically against this base (instead of canonicalizing the
151+
// not-yet-created target file) avoids inconsistent path forms, e.g. on
152+
// Windows mapped network drives where getCanonicalPath() returns the
153+
// UNC form for an existing directory but the drive-letter form for a
154+
// non-existent child, which made a legitimate entry look "outside".
155+
Path destDir = destinationDir.getCanonicalFile().toPath();
156+
Path destFile = destDir.resolve(zipEntry.getName()).normalize();
157+
158+
if (!destFile.startsWith(destDir)) {
154159
throw new IOException("Entry is outside of the target dir: "
155160
+ zipEntry.getName());
156161
}
157162

158-
return destFile;
163+
return destFile.toFile();
159164
}
160165

161166
/**
Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
/*
2+
* Copyright 2000-2026 Vaadin Ltd.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License"); you may not
5+
* use this file except in compliance with the License. You may obtain a copy of
6+
* the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
12+
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
13+
* License for the specific language governing permissions and limitations under
14+
* the License.
15+
*/
16+
package com.vaadin.flow.server.frontend;
17+
18+
import java.io.File;
19+
import java.io.FileOutputStream;
20+
import java.io.IOException;
21+
import java.nio.charset.StandardCharsets;
22+
import java.nio.file.Files;
23+
import java.util.zip.ZipEntry;
24+
import java.util.zip.ZipOutputStream;
25+
26+
import org.junit.Assert;
27+
import org.junit.Rule;
28+
import org.junit.Test;
29+
import org.junit.rules.TemporaryFolder;
30+
31+
public class CompressUtilTest {
32+
33+
@Rule
34+
public TemporaryFolder temporaryFolder = new TemporaryFolder();
35+
36+
@Test
37+
public void uncompressFile_legitimateNestedEntry_isExtracted()
38+
throws IOException {
39+
// Mirrors a real prod.bundle: a nested file entry without an explicit
40+
// directory entry for its parent folder.
41+
File zip = temporaryFolder.newFile("prod.bundle");
42+
try (ZipOutputStream zipOut = new ZipOutputStream(
43+
new FileOutputStream(zip))) {
44+
zipOut.putNextEntry(new ZipEntry("config/bundle-size.html"));
45+
zipOut.write("<html></html>".getBytes(StandardCharsets.UTF_8));
46+
zipOut.closeEntry();
47+
}
48+
49+
File target = temporaryFolder.newFolder("unpacked");
50+
CompressUtil.uncompressFile(zip, target);
51+
52+
File extracted = new File(target, "config/bundle-size.html");
53+
Assert.assertTrue(
54+
"Legitimate nested entry should have been extracted, but the "
55+
+ "Zip Slip guard rejected it",
56+
extracted.exists());
57+
Assert.assertEquals("<html></html>",
58+
Files.readString(extracted.toPath()));
59+
}
60+
61+
@Test
62+
public void uncompressFile_zipSlipEntry_isRejected() throws IOException {
63+
File zip = temporaryFolder.newFile("evil.bundle");
64+
try (ZipOutputStream zipOut = new ZipOutputStream(
65+
new FileOutputStream(zip))) {
66+
zipOut.putNextEntry(new ZipEntry("../evil.txt"));
67+
zipOut.write("pwned".getBytes(StandardCharsets.UTF_8));
68+
zipOut.closeEntry();
69+
}
70+
71+
File target = temporaryFolder.newFolder("unpacked");
72+
73+
Assert.assertThrows(CompressionException.class,
74+
() -> CompressUtil.uncompressFile(zip, target));
75+
Assert.assertFalse(
76+
"Zip Slip entry must not be written outside the target dir",
77+
new File(target.getParentFile(), "evil.txt").exists());
78+
}
79+
}

0 commit comments

Comments
 (0)