This is an automated email from the ASF dual-hosted git repository.
FrankChen021 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/druid.git
The following commit(s) were added to refs/heads/master by this push:
new 91049cfaa92 fix: guard segment info paths against traversal (#19813)
91049cfaa92 is described below
commit 91049cfaa92fdb463fc51174300cb35e2c8adde9
Author: Frank Chen <[email protected]>
AuthorDate: Wed Aug 5 10:32:08 2026 +0800
fix: guard segment info paths against traversal (#19813)
* Guard segment info paths
* fix: handle invalid segment info paths
---
.../apache/druid/java/util/common/FileUtils.java | 25 +++++++++++
.../druid/java/util/common/FileUtilsTest.java | 49 ++++++++++++++++++++++
.../segment/loading/SegmentLocalCacheManager.java | 23 +++++++---
.../loading/SegmentLocalCacheManagerTest.java | 22 ++++++++++
4 files changed, 114 insertions(+), 5 deletions(-)
diff --git
a/processing/src/main/java/org/apache/druid/java/util/common/FileUtils.java
b/processing/src/main/java/org/apache/druid/java/util/common/FileUtils.java
index 7a302a5f4d2..fee9217db61 100644
--- a/processing/src/main/java/org/apache/druid/java/util/common/FileUtils.java
+++ b/processing/src/main/java/org/apache/druid/java/util/common/FileUtils.java
@@ -42,6 +42,7 @@ import java.nio.channels.FileChannel;
import java.nio.file.AccessDeniedException;
import java.nio.file.FileSystemException;
import java.nio.file.Files;
+import java.nio.file.InvalidPathException;
import java.nio.file.NoSuchFileException;
import java.nio.file.Path;
import java.nio.file.StandardCopyOption;
@@ -446,6 +447,30 @@ public class FileUtils
return new File(parentDirectory).toPath();
}
+ /**
+ * Resolves {@code path} below {@code directory}, rejecting absolute paths
and parent traversal that would escape it.
+ * This is intended for paths containing externally supplied identifiers.
+ */
+ public static File resolveFileWithinDirectory(final File directory, final
String path)
+ {
+ final Path normalizedDirectory =
directory.toPath().toAbsolutePath().normalize();
+ final Path childPath;
+ try {
+ childPath = Path.of(path);
+ }
+ catch (InvalidPathException e) {
+ throw new IAE(e, "Path[%s] is not within directory[%s]", path,
directory);
+ }
+ if (childPath.isAbsolute()) {
+ throw new IAE("Path[%s] is not within directory[%s]", path, directory);
+ }
+ final Path resolvedPath =
normalizedDirectory.resolve(childPath).normalize();
+ if (!resolvedPath.startsWith(normalizedDirectory)) {
+ throw new IAE("Path[%s] is not within directory[%s]", path, directory);
+ }
+ return resolvedPath.toFile();
+ }
+
@SuppressForbidden(reason = "Files#createTempDirectory")
public static File createTempDirInLocation(final Path parentDirectory,
@Nullable final String prefix)
{
diff --git
a/processing/src/test/java/org/apache/druid/java/util/common/FileUtilsTest.java
b/processing/src/test/java/org/apache/druid/java/util/common/FileUtilsTest.java
index d242d1fd211..190c58d1f42 100644
---
a/processing/src/test/java/org/apache/druid/java/util/common/FileUtilsTest.java
+++
b/processing/src/test/java/org/apache/druid/java/util/common/FileUtilsTest.java
@@ -31,6 +31,8 @@ import java.io.File;
import java.io.IOException;
import java.io.RandomAccessFile;
import java.nio.file.Files;
+import java.nio.file.InvalidPathException;
+import java.nio.file.Path;
public class FileUtilsTest
{
@@ -171,6 +173,53 @@ public class FileUtilsTest
Assertions.assertEquals("baz",
StringUtils.fromUtf8(Files.readAllBytes(tmpFile.toPath())));
}
+ @Test
+ public void testResolveFileWithinDirectory()
+ {
+ final File resolved =
FileUtils.resolveFileWithinDirectory(temporaryFolder, "nested/file");
+
+ Assertions.assertEquals(
+ temporaryFolder.toPath().toAbsolutePath().resolve(Path.of("nested",
"file")).normalize(),
+ resolved.toPath()
+ );
+ }
+
+ @Test
+ public void testResolveFileWithinDirectoryRejectsTraversal()
+ {
+ Assertions.assertThrows(
+ IAE.class,
+ () -> FileUtils.resolveFileWithinDirectory(temporaryFolder,
"../outside")
+ );
+ }
+
+ @Test
+ public void testResolveFileWithinDirectoryRejectsAbsolutePath()
+ {
+ Assertions.assertThrows(
+ IAE.class,
+ () -> FileUtils.resolveFileWithinDirectory(
+ temporaryFolder,
+
temporaryFolder.toPath().resolve("inside").toAbsolutePath().toString()
+ )
+ );
+ }
+
+ @Test
+ public void testResolveFileWithinDirectoryRejectsInvalidPath()
+ {
+ final IAE exception = Assertions.assertThrows(
+ IAE.class,
+ () -> FileUtils.resolveFileWithinDirectory(temporaryFolder,
"invalid\0path")
+ );
+
+ Assertions.assertEquals(
+ StringUtils.format("Path[%s] is not within directory[%s]",
"invalid\0path", temporaryFolder),
+ exception.getMessage()
+ );
+ Assertions.assertInstanceOf(InvalidPathException.class,
exception.getCause());
+ }
+
@Test
public void testCreateTempDir() throws IOException
{
diff --git
a/server/src/main/java/org/apache/druid/segment/loading/SegmentLocalCacheManager.java
b/server/src/main/java/org/apache/druid/segment/loading/SegmentLocalCacheManager.java
index fdf919a65d6..a0948431e50 100644
---
a/server/src/main/java/org/apache/druid/segment/loading/SegmentLocalCacheManager.java
+++
b/server/src/main/java/org/apache/druid/segment/loading/SegmentLocalCacheManager.java
@@ -33,6 +33,7 @@ import org.apache.druid.client.DataSegmentAndLoadProfile;
import org.apache.druid.error.DruidException;
import org.apache.druid.guice.annotations.Json;
import org.apache.druid.java.util.common.FileUtils;
+import org.apache.druid.java.util.common.IAE;
import org.apache.druid.java.util.common.ISE;
import org.apache.druid.java.util.common.Stopwatch;
import org.apache.druid.java.util.common.concurrent.Execs;
@@ -417,10 +418,15 @@ public class SegmentLocalCacheManager implements
SegmentCacheManager
return files == null ? new File[0] : files;
}
+ private File getSegmentInfoFile(final DataSegment segment)
+ {
+ return FileUtils.resolveFileWithinDirectory(getEffectiveInfoDir(),
segment.getId().toString());
+ }
+
@Override
public void storeInfoFile(final DataSegment segment) throws IOException
{
- final File segmentInfoCacheFile = new File(getEffectiveInfoDir(),
segment.getId().toString());
+ final File segmentInfoCacheFile = getSegmentInfoFile(segment);
if (!segmentInfoCacheFile.exists()) {
FileUtils.mkdirp(segmentInfoCacheFile.getParentFile());
FileUtils.writeAtomically(
@@ -456,9 +462,16 @@ public class SegmentLocalCacheManager implements
SegmentCacheManager
}
}
- private void deleteSegmentInfoFile(DataSegment segment)
+ private void deleteSegmentInfoFile(final DataSegment segment)
{
- final File segmentInfoCacheFile = new File(getEffectiveInfoDir(),
segment.getId().toString());
+ final File segmentInfoCacheFile;
+ try {
+ segmentInfoCacheFile = getSegmentInfoFile(segment);
+ }
+ catch (IAE e) {
+ log.warn(e, "Refusing to delete info file with invalid path for
segment[%s].", segment.getId());
+ return;
+ }
if (!segmentInfoCacheFile.delete()) {
log.warn("Unable to delete cache file[%s] for segment[%s].",
segmentInfoCacheFile, segment.getId());
}
@@ -473,7 +486,7 @@ public class SegmentLocalCacheManager implements
SegmentCacheManager
*/
private void rewriteInfoFile(DataSegment segment) throws IOException
{
- final File segmentInfoCacheFile = new File(getEffectiveInfoDir(),
segment.getId().toString());
+ final File segmentInfoCacheFile = getSegmentInfoFile(segment);
FileUtils.mkdirp(getEffectiveInfoDir());
FileUtils.writeAtomically(segmentInfoCacheFile, out -> {
jsonMapper.writeValue(out, segment);
@@ -530,7 +543,7 @@ public class SegmentLocalCacheManager implements
SegmentCacheManager
try {
if (hold != null) {
// write the segment info file if it doesn't exist. this can
happen if we are loading after a drop
- final File segmentInfoCacheFile = new
File(getEffectiveInfoDir(), dataSegment.getId().toString());
+ final File segmentInfoCacheFile =
getSegmentInfoFile(dataSegment);
if (!segmentInfoCacheFile.exists()) {
FileUtils.mkdirp(getEffectiveInfoDir());
FileUtils.writeAtomically(segmentInfoCacheFile, out -> {
diff --git
a/server/src/test/java/org/apache/druid/segment/loading/SegmentLocalCacheManagerTest.java
b/server/src/test/java/org/apache/druid/segment/loading/SegmentLocalCacheManagerTest.java
index d0ec702926b..3c04bc380ae 100644
---
a/server/src/test/java/org/apache/druid/segment/loading/SegmentLocalCacheManagerTest.java
+++
b/server/src/test/java/org/apache/druid/segment/loading/SegmentLocalCacheManagerTest.java
@@ -29,6 +29,7 @@ import org.apache.druid.error.DruidExceptionMatcher;
import org.apache.druid.guice.LocalDataStorageDruidModule;
import org.apache.druid.jackson.SegmentizerModule;
import org.apache.druid.java.util.common.FileUtils;
+import org.apache.druid.java.util.common.IAE;
import org.apache.druid.java.util.common.Intervals;
import org.apache.druid.java.util.emitter.EmittingLogger;
import org.apache.druid.math.expr.ExprMacroTable;
@@ -59,6 +60,7 @@ import org.junit.rules.TemporaryFolder;
import java.io.File;
import java.io.IOException;
+import java.nio.file.Files;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
@@ -247,6 +249,26 @@ public class SegmentLocalCacheManagerTest extends
InitializedNullHandlingTest
Assert.assertFalse(segment3InfoFile.exists());
}
+ @Test
+ public void testSegmentInfoFileRejectsPathTraversal() throws IOException
+ {
+ final DataSegment segment = TestSegmentUtils.makeSegment(
+ "../../outside",
+ "v0",
+ Intervals.of("2014-10-20T00:00:00Z/P1D")
+ );
+ final File infoDir = new File(localSegmentCacheDir, "info_dir");
+ final File unsafeInfoFile = new File(infoDir, segment.getId().toString());
+ FileUtils.mkdirp(infoDir);
+
+ Assert.assertThrows(IAE.class, () -> manager.storeInfoFile(segment));
+ Assert.assertFalse(unsafeInfoFile.exists());
+
+ Files.write(unsafeInfoFile.toPath(), new byte[]{1});
+ manager.removeInfoFile(segment);
+ Assert.assertTrue(unsafeInfoFile.exists());
+ }
+
@Test
public void testGetCachedSegmentsLegacyPathsMigrated() throws Exception
{
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
