Skip to content

Commit 6e2cafb

Browse files
authored
RATIS-2304. SnapshotManager should validate snapshot file path (#1268)
1 parent 0557974 commit 6e2cafb

File tree

3 files changed

+63
-4
lines changed

3 files changed

+63
-4
lines changed

ratis-common/src/main/java/org/apache/ratis/util/FileUtils.java

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,39 @@ static <T> T attempt(CheckedSupplier<T, IOException> op, Supplier<?> name) throw
6060
}
6161
}
6262

63+
/** @return true iff the given dir is an ancestor of the given sub path. */
64+
static boolean isAncestor(File dir, File sub) throws IOException {
65+
Objects.requireNonNull(dir, "dir == null");
66+
Objects.requireNonNull(sub, "sub == null");
67+
68+
String dirPath = dir.getCanonicalPath();
69+
final String subPath = sub.getCanonicalPath();
70+
if (dirPath.equals(subPath)) {
71+
return true;
72+
} else if (!dirPath.endsWith(File.separator)) {
73+
dirPath += File.separator;
74+
}
75+
LOG.debug("dirPath: {}", dirPath);
76+
LOG.debug("subPath: {}", subPath);
77+
return subPath.startsWith(dirPath);
78+
}
79+
80+
/**
81+
* Resolve the full path from the given dir and sub,
82+
* where dir is supposed to be an ancestor of the resolved path.
83+
*
84+
* @return the full path
85+
* @throws IOException if the dir is not an ancestor of the resolved path.
86+
*/
87+
static File resolveFullPath(File dir, String sub) throws IOException {
88+
final File full = new File(dir, sub);
89+
if (!isAncestor(dir, full)) {
90+
throw new IOException("The dir is not an ancestor of the full path: dir=" + dir
91+
+ ", sub=" + sub + ", full=" + full);
92+
}
93+
return full;
94+
}
95+
6396
static void truncateFile(File f, long target) throws IOException {
6497
final long original = f.length();
6598
LogUtils.runAndLog(LOG,

ratis-common/src/test/java/org/apache/ratis/util/TestFileUtils.java

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,32 @@
2626

2727
/** Test methods of {@link FileUtils}. */
2828
public class TestFileUtils extends BaseTest {
29+
@Test
30+
public void testIsAncestor() throws IOException {
31+
runTestIsAncestor(true, "/a", "/a/b");
32+
runTestIsAncestor(true, "/a", "/a/");
33+
runTestIsAncestor(true, "/a", "/a");
34+
runTestIsAncestor(true, "a", "a/b");
35+
runTestIsAncestor(true, "a", "a/");
36+
runTestIsAncestor(true, "a", "a");
37+
38+
runTestIsAncestor(false, "/a", "/c");
39+
runTestIsAncestor(false, "/a", "/abc");
40+
runTestIsAncestor(false, "/a", "/a/../c");
41+
runTestIsAncestor(false, "a", "a/../c");
42+
runTestIsAncestor(false, "a", "/c");
43+
}
44+
45+
static void runTestIsAncestor(boolean expected, String ancestor, String path) throws IOException {
46+
final boolean computed = isAncestor(ancestor, path);
47+
System.out.printf("isAncestor(%2s, %-9s)? %s, expected? %s%n",
48+
ancestor, path, computed, expected);
49+
Assertions.assertSame(expected, computed);
50+
}
51+
52+
static boolean isAncestor(String ancestor, String path) throws IOException {
53+
return FileUtils.isAncestor(new File(ancestor), new File(path));
54+
}
2955

3056
@Test
3157
public void testRenameToCorrupt() throws IOException {

ratis-server/src/main/java/org/apache/ratis/server/storage/SnapshotManager.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import org.apache.ratis.util.MemoizedSupplier;
3232
import org.apache.ratis.util.Preconditions;
3333
import org.apache.ratis.util.StringUtils;
34+
import org.apache.ratis.util.function.CheckedFunction;
3435
import org.slf4j.Logger;
3536
import org.slf4j.LoggerFactory;
3637

@@ -43,7 +44,6 @@
4344
import java.nio.file.StandardOpenOption;
4445
import java.security.MessageDigest;
4546
import java.util.Optional;
46-
import java.util.function.Function;
4747
import java.util.function.Supplier;
4848

4949
/**
@@ -61,7 +61,7 @@ public class SnapshotManager {
6161

6262
private final Supplier<File> snapshotDir;
6363
private final Supplier<File> snapshotTmpDir;
64-
private final Function<FileChunkProto, String> getRelativePath;
64+
private final CheckedFunction<FileChunkProto, String, IOException> getRelativePath;
6565
private MessageDigest digester;
6666

6767
SnapshotManager(RaftPeerId selfId, Supplier<RaftStorageDirectory> dir, StateMachineStorage smStorage) {
@@ -73,7 +73,7 @@ public class SnapshotManager {
7373

7474
final Supplier<Path> smDir = MemoizedSupplier.valueOf(() -> dir.get().getStateMachineDir().toPath());
7575
this.getRelativePath = c -> smDir.get().relativize(
76-
new File(dir.get().getRoot(), c.getFilename()).toPath()).toString();
76+
FileUtils.resolveFullPath(dir.get().getRoot(), c.getFilename()).toPath()).toString();
7777
}
7878

7979
@SuppressWarnings({"squid:S2095"}) // Suppress closeable warning
@@ -121,7 +121,7 @@ public void installSnapshot(InstallSnapshotRequestProto request, StateMachine st
121121
+ " with endIndex >= lastIncludedIndex " + lastIncludedIndex);
122122
}
123123

124-
final File tmpSnapshotFile = new File(tmpDir, getRelativePath.apply(chunk));
124+
final File tmpSnapshotFile = FileUtils.resolveFullPath(tmpDir, getRelativePath.apply(chunk));
125125
FileUtils.createDirectoriesDeleteExistingNonDirectory(tmpSnapshotFile.getParentFile());
126126

127127
try (FileChannel out = open(chunk, tmpSnapshotFile)) {

0 commit comments

Comments
 (0)