From 217b1fe2a2588f96e978c39722614dd3f080cd54 Mon Sep 17 00:00:00 2001 From: Nick Shanin Date: Tue, 29 Sep 2026 01:08:53 -0400 Subject: [PATCH 1/8] SOLR-18178: Fix configset archive path handling --- .../solr/core/FileSystemConfigSetService.java | 2 +- .../handler/configsets/DownloadConfigSet.java | 34 ++++++++---- .../handler/configsets/UploadConfigSet.java | 10 +++- .../core/TestFileSystemConfigSetService.java | 11 ++++ .../configsets/DownloadConfigSetAPITest.java | 53 +++++++++++++++++++ .../configsets/UploadConfigSetAPITest.java | 36 +++++++++++++ .../TestSchemaDesignerConfigSetHelper.java | 7 ++- 7 files changed, 139 insertions(+), 14 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/core/FileSystemConfigSetService.java b/solr/core/src/java/org/apache/solr/core/FileSystemConfigSetService.java index 0baeadd727a5..376e333eb11d 100644 --- a/solr/core/src/java/org/apache/solr/core/FileSystemConfigSetService.java +++ b/solr/core/src/java/org/apache/solr/core/FileSystemConfigSetService.java @@ -309,7 +309,7 @@ public FileVisitResult postVisitDirectory(Path dir, IOException ioException) { if (!relativePath.isEmpty()) { // We always want to have a trailing forward slash on a directory to // match the normalization to forward slashes everywhere. - filePaths.add(relativePath + '/'); + filePaths.add(normalizePathToForwardSlash(relativePath) + '/'); } return FileVisitResult.CONTINUE; } diff --git a/solr/core/src/java/org/apache/solr/handler/configsets/DownloadConfigSet.java b/solr/core/src/java/org/apache/solr/handler/configsets/DownloadConfigSet.java index 729aaf00d914..24925abd7a05 100644 --- a/solr/core/src/java/org/apache/solr/handler/configsets/DownloadConfigSet.java +++ b/solr/core/src/java/org/apache/solr/handler/configsets/DownloadConfigSet.java @@ -100,11 +100,8 @@ public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) if (Files.isHidden(dir)) { return FileVisitResult.SKIP_SUBTREE; } - String dirName = tmpDirectory.relativize(dir).toString(); - if (!dirName.isEmpty()) { - if (!dirName.endsWith("/")) { - dirName += "/"; - } + String dirName = toZipEntryName(tmpDirectory, dir, true); + if (dirName != null) { zipOut.putNextEntry(new ZipEntry(dirName)); zipOut.closeEntry(); } @@ -115,10 +112,12 @@ public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException { if (!Files.isHidden(file)) { - try (InputStream fis = Files.newInputStream(file)) { - ZipEntry zipEntry = new ZipEntry(tmpDirectory.relativize(file).toString()); - zipOut.putNextEntry(zipEntry); - fis.transferTo(zipOut); + String entryName = toZipEntryName(tmpDirectory, file, false); + if (entryName != null) { + try (InputStream fis = Files.newInputStream(file)) { + zipOut.putNextEntry(new ZipEntry(entryName)); + fis.transferTo(zipOut); + } } } return FileVisitResult.CONTINUE; @@ -130,4 +129,21 @@ public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) } return baos.toByteArray(); } + + /** + * Convert a path under {@code root} into a ZIP-legal relative entry name. + * + *

ZIP requires {@code /} separators and forbids a leading slash. Returns {@code null} for the + * root itself so callers skip the nameless {@code /} directory entry. + */ + static String toZipEntryName(Path root, Path path, boolean directory) { + String name = root.relativize(path).toString().replace('\\', '/'); + if (name.isEmpty()) { + return null; + } + if (directory && !name.endsWith("/")) { + name += "/"; + } + return name; + } } diff --git a/solr/core/src/java/org/apache/solr/handler/configsets/UploadConfigSet.java b/solr/core/src/java/org/apache/solr/handler/configsets/UploadConfigSet.java index bb9ca94c761a..66d2be0fb831 100644 --- a/solr/core/src/java/org/apache/solr/handler/configsets/UploadConfigSet.java +++ b/solr/core/src/java/org/apache/solr/handler/configsets/UploadConfigSet.java @@ -102,9 +102,11 @@ public SolrJerseyResponse uploadConfigSet( while (entries.hasMoreElements()) { ZipEntry zipEntry = entries.nextElement(); hasEntry = true; - String filePath = zipEntry.getName(); + String filePath = normalizeZipEntryName(zipEntry.getName()); filesToDelete.remove(filePath); - if (!zipEntry.isDirectory()) { + // Backslashes are invalid as ZIP separators, but older Windows-produced archives may + // contain them. Normalize before handing the path to either config-set implementation. + if (!zipEntry.isDirectory() && !filePath.endsWith("/")) { try (InputStream entryStream = zipFile.getInputStream(zipEntry)) { configSetService.uploadFileToConfig( configSetName, filePath, entryStream.readAllBytes(), true); @@ -130,6 +132,10 @@ public SolrJerseyResponse uploadConfigSet( return response; } + static String normalizeZipEntryName(String entryName) { + return entryName.replace('\\', '/'); + } + @Override @PermissionName(CONFIG_EDIT_PERM) public SolrJerseyResponse uploadConfigSetFile( diff --git a/solr/core/src/test/org/apache/solr/core/TestFileSystemConfigSetService.java b/solr/core/src/test/org/apache/solr/core/TestFileSystemConfigSetService.java index 5c9af89c5b80..272f1b0713b9 100644 --- a/solr/core/src/test/org/apache/solr/core/TestFileSystemConfigSetService.java +++ b/solr/core/src/test/org/apache/solr/core/TestFileSystemConfigSetService.java @@ -132,6 +132,17 @@ public void testUploadAndDeleteConfig() throws IOException { assertFalse(fileSystemConfigSetService.checkConfigExists("copytestconfig")); } + @Test + public void testGetAllConfigFilesUsesForwardSlashesForDirectories() throws IOException { + String configName = "nestedconfig"; + fileSystemConfigSetService.uploadFileToConfig( + configName, "lang/stopwords_en.txt", "a\nthe".getBytes(StandardCharsets.UTF_8), true); + + assertEquals( + List.of("lang/", "lang/stopwords_en.txt"), + fileSystemConfigSetService.getAllConfigFiles(configName)); + } + private static List getFileList(Path confDir) throws IOException { try (Stream configs = Files.list(confDir)) { return configs diff --git a/solr/core/src/test/org/apache/solr/handler/configsets/DownloadConfigSetAPITest.java b/solr/core/src/test/org/apache/solr/handler/configsets/DownloadConfigSetAPITest.java index 31bfd1e97d83..758ad1363867 100644 --- a/solr/core/src/test/org/apache/solr/handler/configsets/DownloadConfigSetAPITest.java +++ b/solr/core/src/test/org/apache/solr/handler/configsets/DownloadConfigSetAPITest.java @@ -22,9 +22,12 @@ import static org.mockito.Mockito.when; import jakarta.ws.rs.core.Response; +import java.io.ByteArrayInputStream; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; +import java.util.zip.ZipEntry; +import java.util.zip.ZipInputStream; import org.apache.solr.SolrTestCase; import org.apache.solr.common.SolrException; import org.apache.solr.core.CoreContainer; @@ -92,4 +95,54 @@ public void testSuccessfulDownloadReturnsZipResponse() throws Exception { assertNull(response.getHeaderString("Content-Disposition")); } } + + @Test + public void testZipConfigSetUsesForwardSlashEntryNames() throws Exception { + Path configDir = configSetBase.resolve("nestedconfig"); + Files.createDirectories(configDir.resolve("lang")); + Files.writeString(configDir.resolve("solrconfig.xml"), "", StandardCharsets.UTF_8); + Files.writeString( + configDir.resolve("lang").resolve("stopwords_en.txt"), "a\n", StandardCharsets.UTF_8); + + byte[] zipBytes = DownloadConfigSet.zipConfigSet(configSetService, "nestedconfig"); + assertTrue(zipBytes.length > 0); + + boolean foundSolrConfig = false; + boolean foundStopWords = false; + boolean foundLangDir = false; + try (ZipInputStream stream = new ZipInputStream(new ByteArrayInputStream(zipBytes))) { + ZipEntry entry; + while ((entry = stream.getNextEntry()) != null) { + String entryName = entry.getName(); + assertFalse( + "ZIP entry names must use / and must not be empty: " + entryName, entryName.isEmpty()); + assertFalse("ZIP must not include a nameless root directory", "/".equals(entryName)); + assertFalse("ZIP entry names must use / not \\: " + entryName, entryName.contains("\\")); + if ("solrconfig.xml".equals(entryName)) { + foundSolrConfig = true; + } else if ("lang/stopwords_en.txt".equals(entryName)) { + foundStopWords = true; + } else if ("lang/".equals(entryName)) { + foundLangDir = true; + } + } + } + assertTrue("Did not find solrconfig.xml in downloaded configset", foundSolrConfig); + assertTrue("Did not find lang/stopwords_en.txt in downloaded configset", foundStopWords); + assertTrue("Did not find lang/ directory entry in downloaded configset", foundLangDir); + } + + @Test + public void testToZipEntryNameUsesForwardSlashAndSkipsRoot() { + Path root = Path.of("configset-root"); + assertNull(DownloadConfigSet.toZipEntryName(root, root, true)); + assertEquals("lang/", DownloadConfigSet.toZipEntryName(root, root.resolve("lang"), true)); + assertEquals( + "lang/stopwords_en.txt", + DownloadConfigSet.toZipEntryName( + root, root.resolve("lang").resolve("stopwords_en.txt"), false)); + assertEquals( + "lang/stopwords_en.txt", + DownloadConfigSet.toZipEntryName(root, root.resolve("lang\\stopwords_en.txt"), false)); + } } diff --git a/solr/core/src/test/org/apache/solr/handler/configsets/UploadConfigSetAPITest.java b/solr/core/src/test/org/apache/solr/handler/configsets/UploadConfigSetAPITest.java index b376a2592b19..6fdd747bca42 100644 --- a/solr/core/src/test/org/apache/solr/handler/configsets/UploadConfigSetAPITest.java +++ b/solr/core/src/test/org/apache/solr/handler/configsets/UploadConfigSetAPITest.java @@ -27,6 +27,7 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; +import java.util.List; import java.util.zip.ZipEntry; import java.util.zip.ZipOutputStream; import org.apache.solr.SolrTestCase; @@ -304,4 +305,39 @@ public void testZipWithDirectoryEntries() throws Exception { configSetService.downloadFileFromConfig(configSetName, "conf/solrconfig.xml"); assertEquals("", new String(uploadedData, StandardCharsets.UTF_8)); } + + @Test + public void testZipUploadNormalizesBackslashEntryNames() throws Exception { + final String configSetName = "backslashpaths"; + createExistingConfigSet(configSetName, "lang/stopwords/old.txt", "old", "stale.txt", "stale"); + + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + try (ZipOutputStream zos = new ZipOutputStream(baos)) { + zos.putNextEntry(new ZipEntry("lang\\")); + zos.closeEntry(); + zos.putNextEntry(new ZipEntry("lang\\stopwords\\")); + zos.closeEntry(); + zos.putNextEntry(new ZipEntry("lang\\stopwords\\en.txt")); + zos.write("a\nthe".getBytes(StandardCharsets.UTF_8)); + zos.closeEntry(); + } + InputStream zipStream = new ByteArrayInputStream(baos.toByteArray()); + + final var api = new UploadConfigSet(mockCoreContainer, null, null); + api.uploadConfigSet(configSetName, true, true, zipStream); + + byte[] uploadedData = + configSetService.downloadFileFromConfig(configSetName, "lang/stopwords/en.txt"); + assertEquals("a\nthe", new String(uploadedData, StandardCharsets.UTF_8)); + assertEquals( + "lang/stopwords/en.txt", UploadConfigSet.normalizeZipEntryName("lang\\stopwords\\en.txt")); + List configFiles = configSetService.getAllConfigFiles(configSetName); + assertTrue(configFiles.contains("lang/")); + assertTrue(configFiles.contains("lang/stopwords/")); + assertTrue(configFiles.contains("lang/stopwords/en.txt")); + assertTrue(configFiles.stream().noneMatch(path -> path.contains("\\"))); + + assertNull(configSetService.downloadFileFromConfig(configSetName, "lang/stopwords/old.txt")); + assertNull(configSetService.downloadFileFromConfig(configSetName, "stale.txt")); + } } diff --git a/solr/core/src/test/org/apache/solr/handler/designer/TestSchemaDesignerConfigSetHelper.java b/solr/core/src/test/org/apache/solr/handler/designer/TestSchemaDesignerConfigSetHelper.java index 300a78c35812..60fee87cf3cb 100644 --- a/solr/core/src/test/org/apache/solr/handler/designer/TestSchemaDesignerConfigSetHelper.java +++ b/solr/core/src/test/org/apache/solr/handler/designer/TestSchemaDesignerConfigSetHelper.java @@ -128,8 +128,11 @@ public void testDownloadAndZip() throws IOException { ZipEntry entry; while ((entry = stream.getNextEntry()) != null) { - // ZipEntry names have file separators that are OS specific. This normalizes to forward slash. - String entryName = entry.getName().replace('\\', '/'); + String entryName = entry.getName(); + assertFalse( + "ZIP entry names must use / and must not be empty: " + entryName, entryName.isEmpty()); + assertFalse("ZIP must not include a nameless root directory", "/".equals(entryName)); + assertFalse("ZIP entry names must use / not \\: " + entryName, entryName.contains("\\")); if ("solrconfig.xml".equals(entryName)) { foundSolrConfig = true; } else if ("lang/stopwords_en.txt".equals(entryName)) { From fd38d3a91d95afd843783a9218178ec482368ce2 Mon Sep 17 00:00:00 2001 From: Nick Shanin Date: Tue, 29 Sep 2026 21:39:13 -0400 Subject: [PATCH 2/8] SOLR-18178: Add changelog entry --- changelog/unreleased/SOLR-18178.yml | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 changelog/unreleased/SOLR-18178.yml diff --git a/changelog/unreleased/SOLR-18178.yml b/changelog/unreleased/SOLR-18178.yml new file mode 100644 index 000000000000..d9506cf5c45d --- /dev/null +++ b/changelog/unreleased/SOLR-18178.yml @@ -0,0 +1,7 @@ +title: Write portable ZIP entry names when exporting configsets and normalize legacy Windows entries on upload. +type: fixed +authors: + - name: Nick Shanin +links: + - name: SOLR-18178 + url: https://issues.apache.org/jira/browse/SOLR-18178 From b785e6947a2bab1ecc632c46e4a0096cc1159918 Mon Sep 17 00:00:00 2001 From: Nick Shanin Date: Thu, 1 Oct 2026 12:23:01 -0600 Subject: [PATCH 3/8] SOLR-18178: Add negative ZIP-path traversal upload test UploadConfigSet now normalizes backslashes before handing entry names to the config-set backends, which turns Windows-style traversal entries into live '../' syntax on every platform. Pin that both '../evil.txt' and '..\evil2.txt' are contained: neither lands in the configset nor escapes next to it on disk, while legitimate entries still upload. --- .../configsets/UploadConfigSetAPITest.java | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/solr/core/src/test/org/apache/solr/handler/configsets/UploadConfigSetAPITest.java b/solr/core/src/test/org/apache/solr/handler/configsets/UploadConfigSetAPITest.java index 6fdd747bca42..56b04471d83e 100644 --- a/solr/core/src/test/org/apache/solr/handler/configsets/UploadConfigSetAPITest.java +++ b/solr/core/src/test/org/apache/solr/handler/configsets/UploadConfigSetAPITest.java @@ -340,4 +340,41 @@ public void testZipUploadNormalizesBackslashEntryNames() throws Exception { assertNull(configSetService.downloadFileFromConfig(configSetName, "lang/stopwords/old.txt")); assertNull(configSetService.downloadFileFromConfig(configSetName, "stale.txt")); } + + @Test + public void testZipUploadRejectsPathTraversalEntries() throws Exception { + final String configSetName = "traversalpaths"; + createExistingConfigSet(configSetName, "conf/solrconfig.xml", ""); + + // The backslash entry is normalized to forward slashes before the config-set + // backend sees it, so both entries below are traversal attempts on every platform. + // The "conf/" directory entry keeps cleanup from recursively deleting the + // pre-existing conf/ dir (and the new file in it) afterwards. + InputStream zipStream = + createZipStream( + "../evil.txt", + "evil", + "..\\evil2.txt", + "evil2", + "conf/", + "", + "conf/good.txt", + "good"); + + final var api = new UploadConfigSet(mockCoreContainer, null, null); + api.uploadConfigSet(configSetName, true, true, zipStream); + + // Traversal entries must not land in the configset ... + assertNull(configSetService.downloadFileFromConfig(configSetName, "evil.txt")); + assertNull(configSetService.downloadFileFromConfig(configSetName, "evil2.txt")); + // ... nor escape next to it on disk ... + assertFalse(Files.exists(configSetBase.resolve("evil.txt"))); + assertFalse(Files.exists(configSetBase.resolve("evil2.txt"))); + // ... while legitimate entries still upload. + assertEquals( + "good", + new String( + configSetService.downloadFileFromConfig(configSetName, "conf/good.txt"), + StandardCharsets.UTF_8)); + } } From 0e62ba9f27b19a6163eaffcbbc2d9f98d770c58f Mon Sep 17 00:00:00 2001 From: Nick Shanin Date: Sat, 3 Oct 2026 11:01:49 -0600 Subject: [PATCH 4/8] SOLR-18178: reject unsafe zip entry paths before configset backend dispatch UploadConfigSet normalizes backslashes in zip entry names and hands the result to whichever ConfigSetService backend is configured. The filesystem backend contains traversal entries on its own, but the ZooKeeper backend builds a znode path from the entry name, so a single '../' entry aborts the whole upload in SolrCloud instead of being contained. Skip entries whose normalized path is absolute or contains '.' or '..' segments at dispatch, before either backend sees them. Adds a backend-agnostic test that pins exactly which entry names reach a ConfigSetService: only the legitimate entry is dispatched. The test fails against the unpatched dispatch. --- .../handler/configsets/UploadConfigSet.java | 28 +++++++++++++ .../configsets/UploadConfigSetAPITest.java | 42 +++++++++++++++++++ 2 files changed, 70 insertions(+) diff --git a/solr/core/src/java/org/apache/solr/handler/configsets/UploadConfigSet.java b/solr/core/src/java/org/apache/solr/handler/configsets/UploadConfigSet.java index 66d2be0fb831..3244245acedd 100644 --- a/solr/core/src/java/org/apache/solr/handler/configsets/UploadConfigSet.java +++ b/solr/core/src/java/org/apache/solr/handler/configsets/UploadConfigSet.java @@ -106,6 +106,16 @@ public SolrJerseyResponse uploadConfigSet( filesToDelete.remove(filePath); // Backslashes are invalid as ZIP separators, but older Windows-produced archives may // contain them. Normalize before handing the path to either config-set implementation. + if (!isSafeZipEntryPath(filePath)) { + // The filesystem backend contains such entries on its own, but the ZooKeeper + // backend builds a znode path from the entry name and the upload aborts. Reject + // unsafe entry paths here, before either backend sees them. + log.warn( + "Not uploading file [{}] from the uploaded zip, as its path could resolve" + + " outside of the configset root directory", + filePath); + continue; + } if (!zipEntry.isDirectory() && !filePath.endsWith("/")) { try (InputStream entryStream = zipFile.getInputStream(zipEntry)) { configSetService.uploadFileToConfig( @@ -136,6 +146,24 @@ static String normalizeZipEntryName(String entryName) { return entryName.replace('\\', '/'); } + /** + * Whether a normalized zip entry path stays inside the configset it is uploaded to. Absolute + * paths and paths with "." or ".." segments are unsafe: the filesystem backend would resolve + * them outside of the configset directory, and the ZooKeeper backend cannot use them as znode + * path segments at all. + */ + static boolean isSafeZipEntryPath(String normalizedPath) { + if (normalizedPath.startsWith("/")) { + return false; + } + for (String segment : normalizedPath.split("/")) { + if (segment.equals(".") || segment.equals("..")) { + return false; + } + } + return true; + } + @Override @PermissionName(CONFIG_EDIT_PERM) public SolrJerseyResponse uploadConfigSetFile( diff --git a/solr/core/src/test/org/apache/solr/handler/configsets/UploadConfigSetAPITest.java b/solr/core/src/test/org/apache/solr/handler/configsets/UploadConfigSetAPITest.java index 56b04471d83e..b83c53a754d5 100644 --- a/solr/core/src/test/org/apache/solr/handler/configsets/UploadConfigSetAPITest.java +++ b/solr/core/src/test/org/apache/solr/handler/configsets/UploadConfigSetAPITest.java @@ -18,7 +18,13 @@ package org.apache.solr.handler.configsets; import static org.apache.solr.SolrTestCaseJ4.assumeWorkingMockito; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import java.io.ByteArrayInputStream; @@ -32,11 +38,13 @@ import java.util.zip.ZipOutputStream; import org.apache.solr.SolrTestCase; import org.apache.solr.common.SolrException; +import org.apache.solr.core.ConfigSetService; import org.apache.solr.core.CoreContainer; import org.apache.solr.core.FileSystemConfigSetService; import org.junit.Before; import org.junit.BeforeClass; import org.junit.Test; +import org.mockito.ArgumentCaptor; /** Unit tests for {@link UploadConfigSet#uploadConfigSet} (Upload interface). */ public class UploadConfigSetAPITest extends SolrTestCase { @@ -377,4 +385,38 @@ public void testZipUploadRejectsPathTraversalEntries() throws Exception { configSetService.downloadFileFromConfig(configSetName, "conf/good.txt"), StandardCharsets.UTF_8)); } + + @Test + public void testZipUploadSkipsUnsafeEntryPathsBeforeBackendDispatch() throws Exception { + // The traversal guard must hold for every ConfigSetService backend, not just the + // filesystem one: the ZooKeeper backend builds a znode path from the entry name, so an + // entry the dispatch lets through aborts the whole upload there. Use a mock backend and + // pin exactly which entry names the dispatch hands over. + ConfigSetService mockService = mock(ConfigSetService.class); + when(mockService.checkConfigExists(anyString())).thenReturn(false); + CoreContainer container = mock(CoreContainer.class); + when(container.getConfigSetService()).thenReturn(mockService); + + final String configSetName = "anybackend"; + InputStream zipStream = + createZipStream( + "../evil.txt", + "evil", + "..\\evil2.txt", + "evil2", + "/abs.txt", + "abs", + "conf/../evil3.txt", + "evil3", + "conf/good.txt", + "good"); + + final var api = new UploadConfigSet(container, null, null); + api.uploadConfigSet(configSetName, true, false, zipStream); + + ArgumentCaptor fileNameCaptor = ArgumentCaptor.forClass(String.class); + verify(mockService, times(1)) + .uploadFileToConfig(eq(configSetName), fileNameCaptor.capture(), any(), eq(true)); + assertEquals(List.of("conf/good.txt"), fileNameCaptor.getAllValues()); + } } From ccd0a781b971ffb905a4771424e9372553ea3d1f Mon Sep 17 00:00:00 2001 From: Nick Shanin Date: Sat, 3 Oct 2026 11:03:24 -0600 Subject: [PATCH 5/8] SOLR-18178: spotless formatting for the dispatch guard --- .../solr/handler/configsets/UploadConfigSet.java | 6 +++--- .../handler/configsets/UploadConfigSetAPITest.java | 10 +--------- 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/handler/configsets/UploadConfigSet.java b/solr/core/src/java/org/apache/solr/handler/configsets/UploadConfigSet.java index 3244245acedd..43405dfece2d 100644 --- a/solr/core/src/java/org/apache/solr/handler/configsets/UploadConfigSet.java +++ b/solr/core/src/java/org/apache/solr/handler/configsets/UploadConfigSet.java @@ -148,9 +148,9 @@ static String normalizeZipEntryName(String entryName) { /** * Whether a normalized zip entry path stays inside the configset it is uploaded to. Absolute - * paths and paths with "." or ".." segments are unsafe: the filesystem backend would resolve - * them outside of the configset directory, and the ZooKeeper backend cannot use them as znode - * path segments at all. + * paths and paths with "." or ".." segments are unsafe: the filesystem backend would resolve them + * outside of the configset directory, and the ZooKeeper backend cannot use them as znode path + * segments at all. */ static boolean isSafeZipEntryPath(String normalizedPath) { if (normalizedPath.startsWith("/")) { diff --git a/solr/core/src/test/org/apache/solr/handler/configsets/UploadConfigSetAPITest.java b/solr/core/src/test/org/apache/solr/handler/configsets/UploadConfigSetAPITest.java index b83c53a754d5..efd589081491 100644 --- a/solr/core/src/test/org/apache/solr/handler/configsets/UploadConfigSetAPITest.java +++ b/solr/core/src/test/org/apache/solr/handler/configsets/UploadConfigSetAPITest.java @@ -19,7 +19,6 @@ import static org.apache.solr.SolrTestCaseJ4.assumeWorkingMockito; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; @@ -360,14 +359,7 @@ public void testZipUploadRejectsPathTraversalEntries() throws Exception { // pre-existing conf/ dir (and the new file in it) afterwards. InputStream zipStream = createZipStream( - "../evil.txt", - "evil", - "..\\evil2.txt", - "evil2", - "conf/", - "", - "conf/good.txt", - "good"); + "../evil.txt", "evil", "..\\evil2.txt", "evil2", "conf/", "", "conf/good.txt", "good"); final var api = new UploadConfigSet(mockCoreContainer, null, null); api.uploadConfigSet(configSetName, true, true, zipStream); From c3b6ded623e70477d4895d9d393ae3622bd34e45 Mon Sep 17 00:00:00 2001 From: Nick Shanin Date: Sat, 3 Oct 2026 23:12:19 -0600 Subject: [PATCH 6/8] SOLR-18178: Reject drive-qualified and empty zip entry paths isSafeZipEntryPath accepted a normalized entry such as C:/outside/evil.txt: it does not start with a slash and has no dot segments, yet it is absolute on Windows and reached the config-set backend dispatch, outside the guard this change added. Empty entry names passed the same check. The guard now rejects both on every platform (a colon cannot appear in a Windows file name, so a leading drive designator is never a legitimate entry name). Tests pin the guard directly and cover both spellings through the filesystem backend and a mock backend dispatch. --- .../handler/configsets/UploadConfigSet.java | 17 ++++-- .../configsets/UploadConfigSetAPITest.java | 55 +++++++++++++++++++ 2 files changed, 68 insertions(+), 4 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/handler/configsets/UploadConfigSet.java b/solr/core/src/java/org/apache/solr/handler/configsets/UploadConfigSet.java index 43405dfece2d..062158518584 100644 --- a/solr/core/src/java/org/apache/solr/handler/configsets/UploadConfigSet.java +++ b/solr/core/src/java/org/apache/solr/handler/configsets/UploadConfigSet.java @@ -148,12 +148,21 @@ static String normalizeZipEntryName(String entryName) { /** * Whether a normalized zip entry path stays inside the configset it is uploaded to. Absolute - * paths and paths with "." or ".." segments are unsafe: the filesystem backend would resolve them - * outside of the configset directory, and the ZooKeeper backend cannot use them as znode path - * segments at all. + * paths, drive-qualified Windows paths, empty names, and paths with "." or ".." segments are + * unsafe: the filesystem backend would resolve them outside of the configset directory, and the + * ZooKeeper backend cannot use them as znode path segments at all. */ static boolean isSafeZipEntryPath(String normalizedPath) { - if (normalizedPath.startsWith("/")) { + if (normalizedPath.isEmpty() || normalizedPath.startsWith("/")) { + return false; + } + // A drive-qualified name such as "C:/..." does not start with "/" but is still + // absolute on Windows. The check must not depend on the host platform, and a colon + // cannot appear in a Windows file name at all, so reject any leading drive + // designator here. + if (normalizedPath.length() >= 2 + && Character.isLetter(normalizedPath.charAt(0)) + && normalizedPath.charAt(1) == ':') { return false; } for (String segment : normalizedPath.split("/")) { diff --git a/solr/core/src/test/org/apache/solr/handler/configsets/UploadConfigSetAPITest.java b/solr/core/src/test/org/apache/solr/handler/configsets/UploadConfigSetAPITest.java index efd589081491..71f6753406c5 100644 --- a/solr/core/src/test/org/apache/solr/handler/configsets/UploadConfigSetAPITest.java +++ b/solr/core/src/test/org/apache/solr/handler/configsets/UploadConfigSetAPITest.java @@ -400,6 +400,12 @@ public void testZipUploadSkipsUnsafeEntryPathsBeforeBackendDispatch() throws Exc "abs", "conf/../evil3.txt", "evil3", + "C:/outside/evil4.txt", + "evil4", + "C:\\outside\\evil5.txt", + "evil5", + "", + "empty", "conf/good.txt", "good"); @@ -411,4 +417,53 @@ public void testZipUploadSkipsUnsafeEntryPathsBeforeBackendDispatch() throws Exc .uploadFileToConfig(eq(configSetName), fileNameCaptor.capture(), any(), eq(true)); assertEquals(List.of("conf/good.txt"), fileNameCaptor.getAllValues()); } + + @Test + public void testZipUploadRejectsDriveQualifiedEntries() throws Exception { + final String configSetName = "drivepaths"; + createExistingConfigSet(configSetName, "conf/solrconfig.xml", ""); + + // Both spellings of a drive-qualified Windows path are absolute there even + // though neither starts with "/"; the backslash form is normalized first. + // The "conf/" directory entry keeps cleanup from recursively deleting the + // pre-existing conf/ dir (and the new file in it) afterwards. + InputStream zipStream = + createZipStream( + "C:/outside/evil.txt", + "evil", + "C:\\outside\\evil2.txt", + "evil2", + "conf/", + "", + "conf/good.txt", + "good"); + + final var api = new UploadConfigSet(mockCoreContainer, null, null); + api.uploadConfigSet(configSetName, true, true, zipStream); + + // Drive-qualified entries must not land in the configset ... + assertNull(configSetService.downloadFileFromConfig(configSetName, "C:/outside/evil.txt")); + assertNull(configSetService.downloadFileFromConfig(configSetName, "C:/outside/evil2.txt")); + // ... nor be written under a literal "C:" directory on this platform ... + assertFalse(Files.exists(configSetBase.resolve(configSetName).resolve("C:"))); + // ... while legitimate entries still upload. + assertEquals( + "good", + new String( + configSetService.downloadFileFromConfig(configSetName, "conf/good.txt"), + StandardCharsets.UTF_8)); + } + + @Test + public void testSafeZipEntryPathRules() { + assertFalse(UploadConfigSet.isSafeZipEntryPath("")); + assertFalse(UploadConfigSet.isSafeZipEntryPath("/abs.txt")); + assertFalse(UploadConfigSet.isSafeZipEntryPath("C:/outside/evil.txt")); + assertFalse( + UploadConfigSet.isSafeZipEntryPath( + UploadConfigSet.normalizeZipEntryName("C:\\outside\\evil.txt"))); + assertFalse(UploadConfigSet.isSafeZipEntryPath("conf/../evil.txt")); + assertTrue(UploadConfigSet.isSafeZipEntryPath("conf/good.txt")); + assertTrue(UploadConfigSet.isSafeZipEntryPath("solrconfig.xml")); + } } From 502a41ef6bad1a439420b14d5a1bb308f70f37c0 Mon Sep 17 00:00:00 2001 From: Nick Shanin Date: Sun, 4 Oct 2026 22:53:10 +0000 Subject: [PATCH 7/8] SOLR-18178: name the unsafe-entry skipping in the changelog title --- changelog/unreleased/SOLR-18178.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog/unreleased/SOLR-18178.yml b/changelog/unreleased/SOLR-18178.yml index d9506cf5c45d..5300f189dbb6 100644 --- a/changelog/unreleased/SOLR-18178.yml +++ b/changelog/unreleased/SOLR-18178.yml @@ -1,4 +1,4 @@ -title: Write portable ZIP entry names when exporting configsets and normalize legacy Windows entries on upload. +title: Write portable ZIP entry names when exporting configsets, normalize legacy Windows entries on upload, and skip unsafe entries on upload. type: fixed authors: - name: Nick Shanin From 61b8f767c34feb2832bf5b3a2e192bf5d63e5186 Mon Sep 17 00:00:00 2001 From: Nick Shanin Date: Sun, 4 Oct 2026 23:18:51 +0000 Subject: [PATCH 8/8] SOLR-18178: cover nested directories in the config file listing test --- .../apache/solr/core/TestFileSystemConfigSetService.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/solr/core/src/test/org/apache/solr/core/TestFileSystemConfigSetService.java b/solr/core/src/test/org/apache/solr/core/TestFileSystemConfigSetService.java index 272f1b0713b9..ed16fc6b30b2 100644 --- a/solr/core/src/test/org/apache/solr/core/TestFileSystemConfigSetService.java +++ b/solr/core/src/test/org/apache/solr/core/TestFileSystemConfigSetService.java @@ -141,6 +141,15 @@ public void testGetAllConfigFilesUsesForwardSlashesForDirectories() throws IOExc assertEquals( List.of("lang/", "lang/stopwords_en.txt"), fileSystemConfigSetService.getAllConfigFiles(configName)); + + // A single directory name contains no separator, so the case above cannot show the + // conversion; a nested directory is what produces a separator inside the returned name. + fileSystemConfigSetService.uploadFileToConfig( + configName, "lang/nested/deep.txt", "deep".getBytes(StandardCharsets.UTF_8), true); + + assertEquals( + List.of("lang/", "lang/nested/", "lang/nested/deep.txt", "lang/stopwords_en.txt"), + fileSystemConfigSetService.getAllConfigFiles(configName)); } private static List getFileList(Path confDir) throws IOException {