From f6812f151878c48fc62fb0f40b8926de5a93a7bd Mon Sep 17 00:00:00 2001
From: Valery Kharseko <vharseko@3a-systems.ru>
Date: Mon, 27 Jul 2026 13:04:19 +0000
Subject: [PATCH] Fix java/zipslip CodeQL alerts by validating archive entry paths (#761)
---
opendj-maven-plugin/src/main/java/org/forgerock/opendj/maven/GenerateConfigMojo.java | 25 ++++++--
opendj-server-legacy/src/test/java/org/opends/server/util/BackupManagerTestCase.java | 45 +++++++++++++++
opendj-server-legacy/src/test/java/org/opends/server/util/TestStaticUtils.java | 71 +++++++++++++++++++++++
opendj-server-legacy/src/main/java/org/opends/quicksetup/util/ZipExtractor.java | 11 +++
opendj-server-legacy/src/main/java/org/opends/server/util/StaticUtils.java | 11 +++
opendj-server-legacy/src/main/java/org/opends/server/util/BackupManager.java | 7 ++
6 files changed, 157 insertions(+), 13 deletions(-)
diff --git a/opendj-maven-plugin/src/main/java/org/forgerock/opendj/maven/GenerateConfigMojo.java b/opendj-maven-plugin/src/main/java/org/forgerock/opendj/maven/GenerateConfigMojo.java
index ece9fa2..6ab98dc 100644
--- a/opendj-maven-plugin/src/main/java/org/forgerock/opendj/maven/GenerateConfigMojo.java
+++ b/opendj-maven-plugin/src/main/java/org/forgerock/opendj/maven/GenerateConfigMojo.java
@@ -25,6 +25,7 @@
import java.io.IOException;
import java.net.JarURLConnection;
import java.net.URL;
+import java.nio.file.Path;
import java.util.Enumeration;
import java.util.LinkedHashMap;
import java.util.LinkedList;
@@ -245,6 +246,16 @@
createTransformTask(inputFactory, output, stylesheet, executor, parameters);
}
+ /** Resolves a file name against an output directory, refusing names which escape it. */
+ private static String safeOutputPath(final String outputDir, final String fileName) throws IOException {
+ final Path outputDirPath = new File(outputDir).toPath().toAbsolutePath().normalize();
+ final Path outputFile = outputDirPath.resolve(fileName).normalize();
+ if (!outputFile.startsWith(outputDirPath)) {
+ throw new IOException("File name '" + fileName + "' is outside of the output directory");
+ }
+ return outputFile.toString();
+ }
+
private void executeTransformXMLDefinitions() throws Exception {
getLog().info("Transforming XML definitions...");
@@ -282,24 +293,24 @@
for (final Map.Entry<String, StreamSourceFactory> entry : componentDescriptors
.entrySet()) {
- final String meta = metaDir + entry.getKey() + "CfgDefn.java";
+ final String meta = safeOutputPath(metaDir, entry.getKey() + "CfgDefn.java");
createTransformTask(entry.getValue(), meta, stylesheetMetaJava, parallelExecutor);
- final String server = serverDir + entry.getKey() + "Cfg.java";
+ final String server = safeOutputPath(serverDir, entry.getKey() + "Cfg.java");
createTransformTask(entry.getValue(), server, stylesheetServerJava,
parallelExecutor);
- final String client = clientDir + entry.getKey() + "CfgClient.java";
+ final String client = safeOutputPath(clientDir, entry.getKey() + "CfgClient.java");
createTransformTask(entry.getValue(), client, stylesheetClientJava,
parallelExecutor);
- final String ldap = ldapProfileDir + entry.getKey() + "CfgDefn.properties";
+ final String ldap = safeOutputPath(ldapProfileDir, entry.getKey() + "CfgDefn.properties");
createTransformTask(entry.getValue(), ldap, stylesheetProfileLDAP, parallelExecutor);
- final String cli = cliProfileDir + entry.getKey() + "CfgDefn.properties";
+ final String cli = safeOutputPath(cliProfileDir, entry.getKey() + "CfgDefn.properties");
createTransformTask(entry.getValue(), cli, stylesheetProfileCLI, parallelExecutor);
- final String i18n = i18nDir + entry.getKey() + "CfgDefn.properties";
+ final String i18n = safeOutputPath(i18nDir, entry.getKey() + "CfgDefn.properties");
createTransformTask(entry.getValue(), i18n, stylesheetMessages, parallelExecutor);
createTransformTask(entry.getValue(), manifest, stylesheetManifest,
@@ -324,7 +335,7 @@
}
}
};
- final String profile = javaDir + "/" + entry.getKey() + "/package-info.java";
+ final String profile = safeOutputPath(javaDir, entry.getKey() + "/package-info.java");
createTransformTask(sourceFactory, profile, entry.getValue(), parallelExecutor,
"type", entry.getKey());
}
diff --git a/opendj-server-legacy/src/main/java/org/opends/quicksetup/util/ZipExtractor.java b/opendj-server-legacy/src/main/java/org/opends/quicksetup/util/ZipExtractor.java
index d351e5a..fe3542d 100644
--- a/opendj-server-legacy/src/main/java/org/opends/quicksetup/util/ZipExtractor.java
+++ b/opendj-server-legacy/src/main/java/org/opends/quicksetup/util/ZipExtractor.java
@@ -13,6 +13,7 @@
*
* Copyright 2007-2008 Sun Microsystems, Inc.
* Portions Copyright 2011-2016 ForgeRock AS.
+ * Portions Copyright 2026 3A Systems, LLC.
*/
package org.opends.quicksetup.util;
@@ -27,6 +28,7 @@
import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.InputStream;
+import java.nio.file.Path;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
@@ -173,6 +175,7 @@
*/
Map<String, List<String>> permissions = new HashMap<>();
permissions.put(getProtectedDirectoryPermissionUnix(), newArrayList(destDir));
+ Path destDirPath = new File(destDir).toPath().toAbsolutePath().normalize();
try {
if(application != null) {
application.checkAbort();
@@ -198,9 +201,13 @@
}
}
if (name != null && name.length() > 0) {
+ Path destination = destDirPath.resolve(name).normalize();
+ if (!destination.startsWith(destDirPath)) {
+ throw new IOException("Zip entry '" + entry.getName()
+ + "' is outside of the destination directory");
+ }
try {
- File destination = new File(destDir, name);
- copyZipEntry(entry, destination, zipIn,
+ copyZipEntry(entry, destination.toFile(), zipIn,
ratioBeforeCompleted, ratioWhenCompleted, permissions);
} catch (IOException ioe) {
throw new ApplicationException(
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/util/BackupManager.java b/opendj-server-legacy/src/main/java/org/opends/server/util/BackupManager.java
index 591e788..98f3522 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/util/BackupManager.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/util/BackupManager.java
@@ -13,6 +13,7 @@
*
* Copyright 2006-2009 Sun Microsystems, Inc.
* Portions Copyright 2013-2016 ForgeRock AS.
+ * Portions Copyright 2026 3A Systems, LLC.
*/
package org.opends.server.util;
@@ -1098,7 +1099,11 @@
private void restoreZipEntry(String zipEntryName, ZipInputStream zipStream, Path restoreDir,
RestoreConfig restoreConfig) throws IOException, DirectoryException
{
- Path fileToRestore = restoreDir.resolve(zipEntryName);
+ Path fileToRestore = restoreDir.resolve(zipEntryName).normalize();
+ if (!fileToRestore.startsWith(restoreDir.normalize()))
+ {
+ throw new IOException("Zip entry '" + zipEntryName + "' is outside of the restore directory");
+ }
ensureFileCanBeRestored(fileToRestore);
try (OutputStream outputStream = new FileOutputStream(fileToRestore.toFile()))
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/util/StaticUtils.java b/opendj-server-legacy/src/main/java/org/opends/server/util/StaticUtils.java
index 0d36598..5e344e8 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/util/StaticUtils.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/util/StaticUtils.java
@@ -13,7 +13,7 @@
*
* Copyright 2006-2010 Sun Microsystems, Inc.
* Portions Copyright 2011-2016 ForgeRock AS.
- * Portions Copyrighted 2026 3A Systems, LLC.
+ * Portions Copyright 2026 3A Systems, LLC.
*/
package org.opends.server.util;
@@ -32,6 +32,7 @@
import java.net.ServerSocket;
import java.net.Socket;
import java.nio.ByteBuffer;
+import java.nio.file.Path;
import java.text.ParseException;
import java.text.SimpleDateFormat;
import java.util.ArrayList;
@@ -2535,12 +2536,18 @@
public static void extractZipArchive(File zipFile, File targetDirectory, List<String> executableDirectories,
List<String> executableFiles) throws IOException
{
+ Path targetDir = targetDirectory.toPath().toAbsolutePath().normalize();
try (ZipInputStream zipStream = new ZipInputStream(new FileInputStream(zipFile)))
{
ZipEntry fileEntry;
while ((fileEntry = zipStream.getNextEntry()) != null)
{
- File targetFile = new File(targetDirectory.getPath(), fileEntry.getName());
+ Path targetPath = targetDir.resolve(fileEntry.getName()).normalize();
+ if (!targetPath.startsWith(targetDir))
+ {
+ throw new IOException("Zip entry '" + fileEntry.getName() + "' is outside of the target directory");
+ }
+ File targetFile = targetPath.toFile();
if (fileEntry.isDirectory())
{
diff --git a/opendj-server-legacy/src/test/java/org/opends/server/util/BackupManagerTestCase.java b/opendj-server-legacy/src/test/java/org/opends/server/util/BackupManagerTestCase.java
index 31b7883..9c80de3 100644
--- a/opendj-server-legacy/src/test/java/org/opends/server/util/BackupManagerTestCase.java
+++ b/opendj-server-legacy/src/test/java/org/opends/server/util/BackupManagerTestCase.java
@@ -12,6 +12,7 @@
* information: "Portions Copyright [year] [name of copyright owner]".
*
* Copyright 2016 ForgeRock AS.
+ * Portions Copyright 2026 3A Systems, LLC.
*/
package org.opends.server.util;
@@ -28,6 +29,8 @@
import java.util.ArrayList;
import java.util.List;
import java.util.ListIterator;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipOutputStream;
import org.forgerock.opendj.ldap.DN;
import org.opends.server.DirectoryServerTestCase;
@@ -35,6 +38,7 @@
import org.opends.server.api.Backupable;
import org.opends.server.types.BackupConfig;
import org.opends.server.types.BackupDirectory;
+import org.opends.server.types.DirectoryException;
import org.opends.server.types.RestoreConfig;
import org.testng.Reporter;
import org.testng.annotations.BeforeClass;
@@ -214,6 +218,47 @@
cleanDirectories(sourceDirectory, backupPath);
}
+ /**
+ * A restore must reject archive entries which resolve outside of the restore
+ * directory (zip slip) and must not write any file outside of it.
+ */
+ @Test
+ public void testRestoreRejectsZipSlipEntries() throws Exception
+ {
+ String label = "zipslip";
+ Path sourceDirectory = createSourceDirectory(label);
+ Backupable backupable = buildBackupable(sourceDirectory, 1);
+ BackupDirectory backupDir = buildBackupDir(label);
+ BackupConfig backupConfig = new BackupConfig(backupDir, BACKUP_ID, false);
+ RestoreConfig restoreConfig = new RestoreConfig(backupDir, BACKUP_ID, false);
+
+ BackupManager backupManager = new BackupManager(BACKEND_ID);
+ backupManager.createBackup(backupable, backupConfig);
+
+ // replace the archive with one containing an entry escaping the restore directory
+ File archiveFile = new File(backupDir.getPath(), getArchiveFileName(BACKUP_ID));
+ try (ZipOutputStream zipOutput = new ZipOutputStream(new FileOutputStream(archiveFile)))
+ {
+ zipOutput.putNextEntry(new ZipEntry("../evil.txt"));
+ zipOutput.write(getBytes("evil"));
+ zipOutput.closeEntry();
+ }
+
+ File escapedFile = new File(sourceDirectory.toFile().getParentFile(), "evil.txt");
+ try
+ {
+ backupManager.restoreBackup(backupable, restoreConfig);
+ fail("Expected restoreBackup to reject a zip entry escaping the restore directory");
+ }
+ catch (DirectoryException expected)
+ {
+ assertThat(expected.getMessage()).contains("evil.txt");
+ }
+ assertThat(escapedFile).doesNotExist();
+
+ cleanDirectories(sourceDirectory, backupDir.getPath());
+ }
+
@Test
public void testCreateDirectoryWithNumericSuffix() throws Exception
{
diff --git a/opendj-server-legacy/src/test/java/org/opends/server/util/TestStaticUtils.java b/opendj-server-legacy/src/test/java/org/opends/server/util/TestStaticUtils.java
index d64edc6..bff1ef4 100644
--- a/opendj-server-legacy/src/test/java/org/opends/server/util/TestStaticUtils.java
+++ b/opendj-server-legacy/src/test/java/org/opends/server/util/TestStaticUtils.java
@@ -13,7 +13,7 @@
*
* Copyright 2006-2009 Sun Microsystems, Inc.
* Portions Copyright 2013-2016 ForgeRock AS.
- * Portions Copyrighted 2026 3A Systems, LLC.
+ * Portions Copyright 2026 3A Systems, LLC.
*/
package org.opends.server.util;
@@ -22,6 +22,7 @@
import java.io.BufferedReader;
import java.io.BufferedWriter;
import java.io.File;
+import java.io.FileOutputStream;
import java.io.FileReader;
import java.io.FileWriter;
import java.io.IOException;
@@ -34,8 +35,11 @@
import java.nio.channels.FileChannel;
import java.nio.channels.FileLock;
import java.text.ParseException;
+import java.util.Collections;
import java.util.LinkedList;
import java.util.List;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipOutputStream;
import org.forgerock.opendj.ldap.ByteString;
import org.opends.server.TestCaseUtils;
@@ -1100,4 +1104,69 @@
Assert.assertTrue(StaticUtils.isSelfConnection(socket));
}
}
+
+ /**
+ * {@link StaticUtils#extractZipArchive} must reject archive entries which
+ * resolve outside of the target directory (zip slip) and must not write any
+ * file outside of it.
+ */
+ @Test
+ public void testExtractZipArchiveRejectsZipSlipEntries() throws Exception
+ {
+ File tempDir = TestCaseUtils.createTemporaryDirectory("zipslip");
+ File zipFile = new File(tempDir, "malicious.zip");
+ try (ZipOutputStream zipOutput = new ZipOutputStream(new FileOutputStream(zipFile)))
+ {
+ zipOutput.putNextEntry(new ZipEntry("../evil.txt"));
+ zipOutput.write(StaticUtils.getBytes("evil"));
+ zipOutput.closeEntry();
+ }
+ File targetDirectory = new File(tempDir, "target");
+ Assert.assertTrue(targetDirectory.mkdirs());
+
+ try
+ {
+ StaticUtils.extractZipArchive(zipFile, targetDirectory,
+ Collections.<String> emptyList(), Collections.<String> emptyList());
+ Assert.fail("Expected extractZipArchive to reject a zip entry escaping the target directory");
+ }
+ catch (IOException expected)
+ {
+ // expected
+ }
+ Assert.assertFalse(new File(tempDir, "evil.txt").exists());
+ }
+
+ /**
+ * {@link StaticUtils#extractZipArchive} must accept a relative target
+ * directory: EmbeddedDirectoryServer passes the parent of a relative server
+ * root, e.g. {@code .} for a server root of {@code ./opendj}.
+ */
+ @Test
+ public void testExtractZipArchiveSupportsRelativeTargetDirectory() throws Exception
+ {
+ File tempDir = TestCaseUtils.createTemporaryDirectory("zipslip-relative");
+ File zipFile = new File(tempDir, "archive.zip");
+ String rootName = "zipslip-relative-extract-" + System.nanoTime();
+ try (ZipOutputStream zipOutput = new ZipOutputStream(new FileOutputStream(zipFile)))
+ {
+ zipOutput.putNextEntry(new ZipEntry(rootName + "/"));
+ zipOutput.closeEntry();
+ zipOutput.putNextEntry(new ZipEntry(rootName + "/README"));
+ zipOutput.write(StaticUtils.getBytes("content"));
+ zipOutput.closeEntry();
+ }
+
+ File extractedRoot = new File(rootName);
+ try
+ {
+ StaticUtils.extractZipArchive(zipFile, new File("."),
+ Collections.<String> emptyList(), Collections.<String> emptyList());
+ Assert.assertTrue(new File(extractedRoot, "README").exists());
+ }
+ finally
+ {
+ StaticUtils.recursiveDelete(extractedRoot);
+ }
+ }
}
--
Gitblit v1.10.0