This is an automated email from the ASF dual-hosted git repository. szetszwo pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/ratis.git
commit 1f4be875d4df922a3389c44b62834a38fdd623a2 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)) {
