mirror of https://github.com/OpenIdentityPlatform/OpenDJ.git

Valery Kharseko
10 hours ago f6812f151878c48fc62fb0f40b8926de5a93a7bd
Fix java/zipslip CodeQL alerts by validating archive entry paths (#761)
6 files modified
170 ■■■■■ changed files
opendj-maven-plugin/src/main/java/org/forgerock/opendj/maven/GenerateConfigMojo.java 25 ●●●● patch | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/quicksetup/util/ZipExtractor.java 11 ●●●● patch | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/server/util/BackupManager.java 7 ●●●● patch | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/server/util/StaticUtils.java 11 ●●●● patch | view | raw | blame | history
opendj-server-legacy/src/test/java/org/opends/server/util/BackupManagerTestCase.java 45 ●●●●● patch | view | raw | blame | history
opendj-server-legacy/src/test/java/org/opends/server/util/TestStaticUtils.java 71 ●●●●● patch | view | raw | blame | history
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());
            }
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(
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()))
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())
        {
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
  {
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);
    }
  }
}