Skip to content

Commit 37ecd84

Browse files
authored
Merge pull request #34504 from odaysec/fix-dev
fix: arbitrary file access during archive extraction zipslip
2 parents 1eb4f58 + 5ff342c commit 37ecd84

7 files changed

Lines changed: 44 additions & 9 deletions

File tree

client/go/internal/vespa/application.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,11 @@ func (ap *ApplicationPackage) Unzip(test bool) (string, error) {
254254
}
255255
defer f.Close()
256256
for _, f := range f.File {
257+
// Normalize the file path and ensure it stays within the temp directory
257258
dst := filepath.Join(tmp, f.Name)
259+
if !strings.HasPrefix(filepath.Clean(dst), filepath.Clean(tmp)+string(os.PathSeparator)) {
260+
return "", fmt.Errorf("illegal file path in archive: %s", f.Name)
261+
}
258262
if f.FileInfo().IsDir() {
259263
if err := os.Mkdir(dst, f.FileInfo().Mode()); err != nil {
260264
return "", err

config-application-package/src/main/java/com/yahoo/config/application/ConfigDefinitionDir.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,13 @@ private void checkAndCopyUserDefs(Bundle bundle, List<Bundle> bundlesAdded) thro
3131
for (Bundle.DefEntry def : bundle.getDefEntries()) {
3232
checkUserDefConflict(bundle, def, bundlesAdded);
3333
String defFilename = def.defNamespace + "." + def.defName + ".def";
34-
OutputStream out = new FileOutputStream(new File(defDir, defFilename));
34+
File outFile = new File(defDir, defFilename);
35+
// Zip Slip fix: ensure output file path is within defDir
36+
if (!outFile.toPath().normalize().startsWith(defDir.toPath().normalize())) {
37+
throw new IllegalArgumentException("Refusing to write config definition outside of " + defDir.getAbsolutePath() +
38+
" (got " + outFile.getAbsolutePath() + ")");
39+
}
40+
OutputStream out = new FileOutputStream(outFile);
3541
out.write(def.contents.getBytes());
3642
out.close();
3743
}

config-application-package/src/main/java/com/yahoo/config/model/application/provider/SchemaValidators.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,13 @@ private static void copySchemas(File from, File to) throws IOException {
169169

170170
private static void writeContentsToFile(File outDir, String outFile, InputStream inputStream) throws IOException {
171171
String contents = IOUtils.readAll(new InputStreamReader(inputStream));
172-
File out = new File(outDir, outFile);
172+
173+
// Validate and sanitize the output path
174+
File out = new File(outDir, outFile).getCanonicalFile();
175+
if (!out.getPath().startsWith(outDir.getCanonicalPath() + File.separator)) {
176+
throw new IOException("Invalid file path: " + outFile);
177+
}
178+
173179
IOUtils.writeFile(out, contents, false);
174180
}
175181

configserver/src/main/java/com/yahoo/vespa/config/server/application/CompressedApplicationInputStream.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,9 @@ private File decompressInto(Path dir) throws IOException {
9191
tmpStream.close();
9292
log.log(Level.FINE, "Creating output file: " + file.path());
9393
Path dstFile = dir.resolve(file.path().toString()).normalize();
94+
if (!dstFile.startsWith(dir.normalize())) {
95+
throw new IOException("Entry attempts to write outside the target directory: " + dstFile);
96+
}
9497
Files.createDirectories(dstFile.getParent());
9598
Files.move(tmpFile, dstFile);
9699
tmpFile = createTempFile(dir);

filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileReferenceCompressor.java

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -71,20 +71,27 @@ private static void decompress(TarArchiveInputStream archiveInputStream, File ou
7171
ArchiveEntry entry;
7272
while ((entry = archiveInputStream.getNextEntry()) != null) {
7373
File outFile = new File(outputFile, entry.getName());
74+
File canonicalOutFile = outFile.getCanonicalFile();
75+
File canonicalOutputDir = outputFile.getCanonicalFile();
76+
77+
if (!canonicalOutFile.toPath().startsWith(canonicalOutputDir.toPath())) {
78+
throw new IOException("Invalid archive entry: " + entry.getName());
79+
}
80+
7481
if (entry.isDirectory()) {
75-
if (!(outFile.exists() && outFile.isDirectory())) {
76-
log.log(Level.FINE, () -> "Creating dir: " + outFile.getAbsolutePath());
77-
if (!outFile.mkdirs()) {
82+
if (!(canonicalOutFile.exists() && canonicalOutFile.isDirectory())) {
83+
log.log(Level.FINE, () -> "Creating dir: " + canonicalOutFile.getAbsolutePath());
84+
if (!canonicalOutFile.mkdirs()) {
7885
log.log(Level.WARNING, "Could not create dir " + entry.getName());
7986
}
8087
}
8188
} else {
8289
// Create parent dir if necessary
83-
File parent = new File(outFile.getParent());
90+
File parent = new File(canonicalOutFile.getParent());
8491
if (!parent.exists() && !parent.mkdirs()) {
8592
log.log(Level.WARNING, "Could not create dir " + parent.getAbsolutePath());
8693
}
87-
try (FileOutputStream fos = new FileOutputStream(outFile)) {
94+
try (FileOutputStream fos = new FileOutputStream(canonicalOutFile)) {
8895
archiveInputStream.transferTo(fos);
8996
}
9097
}

integration/schema-language-server/language-server/src/main/java/ai/vespa/schemals/SchemaLanguageServer.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,11 @@ public boolean accept(File pathname) {
271271
while (entries.hasMoreElements()) {
272272
JarEntry entry = entries.nextElement();
273273
if (!entry.isDirectory() && entry.getName().startsWith(documentationPath.getFileName().toString())) {
274-
Path destination = documentationPath.getParent().resolve(entry.getName());
274+
Path extractionDir = documentationPath.getParent();
275+
Path destination = extractionDir.resolve(entry.getName()).normalize();
276+
if (!destination.startsWith(extractionDir)) {
277+
throw new IOException("Zip entry is outside of the target dir: " + entry.getName());
278+
}
275279
Files.createDirectories(destination.getParent());
276280
try (InputStream inputStream = Thread.currentThread().getContextClassLoader().getResourceAsStream(entry.getName())) {
277281
BufferedReader reader = new BufferedReader(new InputStreamReader(inputStream));

integration/schema-language-server/lemminx-vespa/src/main/java/ai/vespa/lemminx/UnpackRNGFiles.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,12 @@ public static void unpackRNGFiles(Path serverPath) throws IOException {
4242
while (entries.hasMoreElements()) {
4343
JarEntry entry = entries.nextElement();
4444
if (!entry.isDirectory() && entry.getName().endsWith(".rng") && entry.getName().startsWith("resources/schema")) {
45-
Path writePath = serverPath.resolve(entry.getName());
45+
Path writePath = serverPath.resolve(entry.getName()).normalize();
46+
Path targetDir = serverPath.resolve("resources").resolve("schema").normalize();
47+
if (!writePath.startsWith(targetDir)) {
48+
// Potential Zip Slip attack, skip this entry
49+
continue;
50+
}
4651
try (InputStream inputStream = Thread.currentThread().getContextClassLoader().getResourceAsStream(entry.getName())) {
4752
BufferedReader reader = new BufferedReader(new InputStreamReader(inputStream));
4853
String content = reader.lines().collect(Collectors.joining(System.lineSeparator()));

0 commit comments

Comments
 (0)