This is an automated email from the ASF dual-hosted git repository.
adoroszlai pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ratis.git
The following commit(s) were added to refs/heads/master by this push:
new 6e2cafb6b RATIS-2304. SnapshotManager should validate snapshot file
path (#1268)
6e2cafb6b is described below
commit 6e2cafb6b7c18e7322079e0fa0ca2548a3bcd55f
Author: Tsz-Wo Nicholas Sze <[email protected]>
AuthorDate: Thu May 22 00:59:14 2025 -0700
RATIS-2304. SnapshotManager should validate snapshot file path (#1268)
---
.../main/java/org/apache/ratis/util/FileUtils.java | 33 ++++++++++++++++++++++
.../java/org/apache/ratis/util/TestFileUtils.java | 26 +++++++++++++++++
.../ratis/server/storage/SnapshotManager.java | 8 +++---
3 files changed, 63 insertions(+), 4 deletions(-)
diff --git a/ratis-common/src/main/java/org/apache/ratis/util/FileUtils.java
b/ratis-common/src/main/java/org/apache/ratis/util/FileUtils.java
index 4b9d9e3b2..315010c62 100644
--- a/ratis-common/src/main/java/org/apache/ratis/util/FileUtils.java
+++ b/ratis-common/src/main/java/org/apache/ratis/util/FileUtils.java
@@ -60,6 +60,39 @@ public interface FileUtils {
}
}
+ /** @return true iff the given dir is an ancestor of the given sub path. */
+ static boolean isAncestor(File dir, File sub) throws IOException {
+ Objects.requireNonNull(dir, "dir == null");
+ Objects.requireNonNull(sub, "sub == null");
+
+ String dirPath = dir.getCanonicalPath();
+ final String subPath = sub.getCanonicalPath();
+ if (dirPath.equals(subPath)) {
+ return true;
+ } else if (!dirPath.endsWith(File.separator)) {
+ dirPath += File.separator;
+ }
+ LOG.debug("dirPath: {}", dirPath);
+ LOG.debug("subPath: {}", subPath);
+ return subPath.startsWith(dirPath);
+ }
+
+ /**
+ * Resolve the full path from the given dir and sub,
+ * where dir is supposed to be an ancestor of the resolved path.
+ *
+ * @return the full path
+ * @throws IOException if the dir is not an ancestor of the resolved path.
+ */
+ static File resolveFullPath(File dir, String sub) throws IOException {
+ final File full = new File(dir, sub);
+ if (!isAncestor(dir, full)) {
+ throw new IOException("The dir is not an ancestor of the full path:
dir=" + dir
+ + ", sub=" + sub + ", full=" + full);
+ }
+ return full;
+ }
+
static void truncateFile(File f, long target) throws IOException {
final long original = f.length();
LogUtils.runAndLog(LOG,
diff --git
a/ratis-common/src/test/java/org/apache/ratis/util/TestFileUtils.java
b/ratis-common/src/test/java/org/apache/ratis/util/TestFileUtils.java
index 3171756b8..8f64ff294 100644
--- a/ratis-common/src/test/java/org/apache/ratis/util/TestFileUtils.java
+++ b/ratis-common/src/test/java/org/apache/ratis/util/TestFileUtils.java
@@ -26,6 +26,32 @@ import java.io.IOException;
/** Test methods of {@link FileUtils}. */
public class TestFileUtils extends BaseTest {
+ @Test
+ public void testIsAncestor() throws IOException {
+ runTestIsAncestor(true, "/a", "/a/b");
+ runTestIsAncestor(true, "/a", "/a/");
+ runTestIsAncestor(true, "/a", "/a");
+ runTestIsAncestor(true, "a", "a/b");
+ runTestIsAncestor(true, "a", "a/");
+ runTestIsAncestor(true, "a", "a");
+
+ runTestIsAncestor(false, "/a", "/c");
+ runTestIsAncestor(false, "/a", "/abc");
+ runTestIsAncestor(false, "/a", "/a/../c");
+ runTestIsAncestor(false, "a", "a/../c");
+ runTestIsAncestor(false, "a", "/c");
+ }
+
+ static void runTestIsAncestor(boolean expected, String ancestor, String
path) throws IOException {
+ final boolean computed = isAncestor(ancestor, path);
+ System.out.printf("isAncestor(%2s, %-9s)? %s, expected? %s%n",
+ ancestor, path, computed, expected);
+ Assertions.assertSame(expected, computed);
+ }
+
+ static boolean isAncestor(String ancestor, String path) throws IOException {
+ return FileUtils.isAncestor(new File(ancestor), new File(path));
+ }
@Test
public void testRenameToCorrupt() throws IOException {
diff --git
a/ratis-server/src/main/java/org/apache/ratis/server/storage/SnapshotManager.java
b/ratis-server/src/main/java/org/apache/ratis/server/storage/SnapshotManager.java
index 91c1ba5b9..2d10c53a4 100644
---
a/ratis-server/src/main/java/org/apache/ratis/server/storage/SnapshotManager.java
+++
b/ratis-server/src/main/java/org/apache/ratis/server/storage/SnapshotManager.java
@@ -31,6 +31,7 @@ import org.apache.ratis.util.MD5FileUtil;
import org.apache.ratis.util.MemoizedSupplier;
import org.apache.ratis.util.Preconditions;
import org.apache.ratis.util.StringUtils;
+import org.apache.ratis.util.function.CheckedFunction;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -43,7 +44,6 @@ import java.nio.file.Path;
import java.nio.file.StandardOpenOption;
import java.security.MessageDigest;
import java.util.Optional;
-import java.util.function.Function;
import java.util.function.Supplier;
/**
@@ -61,7 +61,7 @@ public class SnapshotManager {
private final Supplier<File> snapshotDir;
private final Supplier<File> snapshotTmpDir;
- private final Function<FileChunkProto, String> getRelativePath;
+ private final CheckedFunction<FileChunkProto, String, IOException>
getRelativePath;
private MessageDigest digester;
SnapshotManager(RaftPeerId selfId, Supplier<RaftStorageDirectory> dir,
StateMachineStorage smStorage) {
@@ -73,7 +73,7 @@ public class SnapshotManager {
final Supplier<Path> smDir = MemoizedSupplier.valueOf(() ->
dir.get().getStateMachineDir().toPath());
this.getRelativePath = c -> smDir.get().relativize(
- new File(dir.get().getRoot(), c.getFilename()).toPath()).toString();
+ FileUtils.resolveFullPath(dir.get().getRoot(),
c.getFilename()).toPath()).toString();
}
@SuppressWarnings({"squid:S2095"}) // Suppress closeable warning
@@ -121,7 +121,7 @@ public class SnapshotManager {
+ " with endIndex >= lastIncludedIndex " + lastIncludedIndex);
}
- final File tmpSnapshotFile = new File(tmpDir,
getRelativePath.apply(chunk));
+ final File tmpSnapshotFile = FileUtils.resolveFullPath(tmpDir,
getRelativePath.apply(chunk));
FileUtils.createDirectoriesDeleteExistingNonDirectory(tmpSnapshotFile.getParentFile());
try (FileChannel out = open(chunk, tmpSnapshotFile)) {
