Tighten up processing of xml, zip file entries

cdm/core/src/main/java/ucar/nc2/ft/remote/CdmrFeatureDataset.java

@@ -118,7 +118,10 @@ private static org.jdom2.Document getCapabilities(String endpoint) throws IOExce
     org.jdom2.Document doc;
     try (InputStream in = CdmRemote.sendQuery(null, endpoint, "req=capabilities")) {
       SAXBuilder builder = new SAXBuilder();
+      // this is the same as builder.setFeature("http://xml.org/sax/features/external-general-entities", false);
       builder.setExpandEntities(false);
+      builder.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
+      builder.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
       doc = builder.build(in); // LOOK closes in when done ??
 
     } catch (Throwable t) {

cdm/zarr/src/main/java/thredds/filesystem/zarr/ControllerZip.java

@@ -123,11 +123,17 @@ protected static class FilteredIterator extends MFileIterator implements Iterato
       Path relativePath = file.getRelativePath();
 
       for (ZipEntry entry : entries) {
-        Path entryPath = Paths.get(File.separator + entry.getName());
-        if (!entryPath.startsWith(relativePath)) {
-          logger.warn(entryPath.toString() + " is not an entry in " + relativePath.toString());
+        // entry path (relative to zip file), normalized to remove
+        // segments like '.' and '..'
+        Path normEntryPath = Paths.get(entry.getName()).normalize();
+        // if normEntryPath starts with '..', it has tried to escape
+        // above its original starting level
+        if (normEntryPath.startsWith("..")) {
+          logger.warn(normEntryPath.toString() + " is not an entry in " + relativePath.toString());
           continue;
         }
+        // anchor entry path to root of zip file
+        Path entryPath = Paths.get(File.separator + entry.getName());
         // truncate path to one level below current path (i.e. direct child)
         Path childPath = entryPath.subpath(0, relativePath.getNameCount() + 1);
         fileNames.add(childPath);
@@ -155,7 +161,14 @@ protected static class MFileIteratorLeaves extends MFileIterator implements Iter
       List<ZipEntry> entries = file.getLeafEntries();
       for (ZipEntry entry : entries) {
         try {
-          this.files.add(new MFileZip(file.getRootPath() + File.separator + entry.getName()));
+          File entryFile = new File(file.getRootPath() + File.separator + entry.getName());
+          if (entryFile.toPath().normalize().startsWith(file.getRootPath())) {
+            this.files.add(new MFileZip(entryFile.toString()));
+          } else {
+            // don't allow external references to escape the zip file
+            // (e.g., skip entries like ../path/outside/of/zip)
+            logger.warn("Zip entry references external entity in {}: {}. Skipping.", file.getPath(), entryFile);
+          }
         } catch (IOException ioe) {
           logger.error(ioe.getMessage(), ioe);
         }

cdm/zarr/src/test/java/thredds/filesystem/zarr/TestControllerZip.java

@@ -0,0 +1,136 @@
+/*
+ * Copyright (c) 2026 University Corporation for Atmospheric Research/Unidata
+ * See LICENSE for license information.
+ */
+
+package thredds.filesystem.zarr;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import java.nio.file.Files;
+import java.util.Arrays;
+import java.util.Collections;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import thredds.inventory.CollectionConfig;
+import thredds.inventory.MFile;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.DirectoryStream;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipOutputStream;
+
+public class TestControllerZip {
+
+  @ClassRule
+  public static final TemporaryFolder tempFolder = new TemporaryFolder();
+
+  private static File zipFile, zipFileBad;
+
+  @BeforeClass
+  public static void setUp() throws IOException {
+    zipFile = tempFolder.newFile("test.zip");
+    try (ZipOutputStream zos = new ZipOutputStream(Files.newOutputStream(zipFile.toPath()))) {
+      // file1 (top level)
+      zos.putNextEntry(new ZipEntry("file1"));
+      zos.write("content1".getBytes());
+      zos.closeEntry();
+
+      // dir1/file2
+      zos.putNextEntry(new ZipEntry("dir1/file2"));
+      zos.write("content2".getBytes());
+      zos.closeEntry();
+
+      // dir1/file3
+      zos.putNextEntry(new ZipEntry("dir1/file3"));
+      zos.write("content3".getBytes());
+      zos.closeEntry();
+    }
+
+    zipFileBad = tempFolder.newFile("test_bad.zip");
+    try (ZipOutputStream zos = new ZipOutputStream(Files.newOutputStream(zipFileBad.toPath()))) {
+      // good entries
+      zos.putNextEntry(new ZipEntry("dir1/file_good1"));
+      zos.write("content1".getBytes());
+      zos.closeEntry();
+
+      zos.putNextEntry(new ZipEntry("dir1/file_good2"));
+      zos.write("content2".getBytes());
+      zos.closeEntry();
+
+      // bad entries
+      // reference outside of zip
+      zos.putNextEntry(new ZipEntry("../../file_bad"));
+      zos.write("content3".getBytes());
+      zos.closeEntry();
+
+      // reference outside of zip
+      zos.putNextEntry(new ZipEntry("dir3/../../file_bad2"));
+      zos.write("content4".getBytes());
+      zos.closeEntry();
+    }
+  }
+
+  @Test
+  public void testFilteredIteratorFiles() throws IOException {
+    ControllerZip controller = new ControllerZip();
+    CollectionConfig mc = new CollectionConfig("test", zipFile.getAbsolutePath(), false, null, null);
+    try (DirectoryStream<MFile> stream = controller.getInventoryTop(mc, false)) {
+      assertThat(stream).isNotNull();
+      List<String> names = new ArrayList<>();
+      for (MFile mfile : stream) {
+        names.add(mfile.getName());
+      }
+      // only one item in the top level of the zip
+      assertThat(names).containsExactly(File.separator + "file1");
+    }
+  }
+
+  @Test
+  public void testFilteredIteratorDirs() throws IOException {
+    ControllerZip controller = new ControllerZip();
+    CollectionConfig mc = new CollectionConfig("test", zipFile.getAbsolutePath(), false, null, null);
+    try (DirectoryStream<MFile> stream = controller.getSubdirs(mc, false)) {
+      assertThat(stream).isNotNull();
+      List<String> names = new ArrayList<>();
+      for (MFile mfile : stream) {
+        names.add(mfile.getName());
+      }
+      assertThat(names).containsExactly(File.separator + "dir1");
+    }
+  }
+
+  @Test
+  public void testFilteredFilesBad() throws IOException {
+    ControllerZip controller = new ControllerZip();
+    CollectionConfig mc = new CollectionConfig("test", zipFileBad.getAbsolutePath(), false, null, null);
+    try (DirectoryStream<MFile> stream = controller.getInventoryAll(mc, false)) {
+      assertThat(stream).isNotNull();
+      List<String> names = new ArrayList<>();
+      for (MFile mfile : stream) {
+        names.add(mfile.getName());
+      }
+      assertThat(names).containsExactlyElementsIn(
+          Arrays.asList(File.separator + "dir1/file_good1", File.separator + "dir1/file_good2"));
+    }
+  }
+
+  @Test
+  public void testFilteredIteratorDirsBad() throws IOException {
+    ControllerZip controller = new ControllerZip();
+    CollectionConfig mc = new CollectionConfig("test", zipFileBad.getAbsolutePath(), false, null, null);
+    try (DirectoryStream<MFile> stream = controller.getSubdirs(mc, false)) {
+      assertThat(stream).isNotNull();
+      List<String> names = new ArrayList<>();
+      for (MFile mfile : stream) {
+        names.add(mfile.getName());
+      }
+      assertThat(names).containsExactlyElementsIn(Collections.singletonList(File.separator + "dir1"));
+    }
+  }
+}

opendap/src/main/java/opendap/dap/parsers/DDSXMLParser.java

@@ -219,7 +219,10 @@ public void parse(InputStream is, DDS targetDDS, BaseTypeFactory fac, boolean va
 
       // get a jdom parser to parse and validate the XML document.
       SAXBuilder parser = new SAXBuilder();
+      // this is the same as builder.setFeature("http://xml.org/sax/features/external-general-entities", false);
       parser.setExpandEntities(false);
+      parser.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
+      parser.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
       // optionally turn on validation
       parser.setFeature("http://apache.org/xml/features/validation/schema", validation);
 

uicdm/src/main/java/ucar/nc2/ui/op/WmsViewer.java

@@ -190,7 +190,10 @@ private boolean getCapabilities() {
       }
 
       SAXBuilder builder = new SAXBuilder();
+      // this is the same as builder.setFeature("http://xml.org/sax/features/external-general-entities", false);
       builder.setExpandEntities(false);
+      builder.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
+      builder.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
       Document tdoc = builder.build(method.getResponseAsStream());
       Element root = tdoc.getRootElement();
       parseGetCapabilities(root);