Skip to content

Commit cfd214b

Browse files
committed
prevent symlink loop recursion in directory traversal
1 parent a63c5f0 commit cfd214b

2 files changed

Lines changed: 72 additions & 11 deletions

File tree

fuzzing/replay/file_util.cc

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,9 @@
2121

2222
#include <cerrno>
2323
#include <cstdio>
24+
#include <set>
2425
#include <string>
26+
#include <utility>
2527

2628
#include "absl/functional/function_ref.h"
2729
#include "absl/status/status.h"
@@ -36,7 +38,33 @@ namespace {
3638

3739
absl::Status TraverseDirectory(
3840
absl::string_view path,
39-
absl::FunctionRef<void(absl::string_view, const struct stat&)> callback) {
41+
absl::FunctionRef<void(absl::string_view, const struct stat&)> callback,
42+
std::set<std::pair<dev_t, ino_t>>& visited);
43+
44+
absl::Status YieldFilesInternal(
45+
absl::string_view path,
46+
absl::FunctionRef<void(absl::string_view, const struct stat&)> callback,
47+
std::set<std::pair<dev_t, ino_t>>& visited) {
48+
struct stat path_stat;
49+
if (stat(std::string(path).c_str(), &path_stat) < 0) {
50+
return ErrnoStatus(absl::StrCat("could not stat ", path), errno);
51+
}
52+
if (S_ISDIR(path_stat.st_mode)) {
53+
auto dir_id = std::make_pair(path_stat.st_dev, path_stat.st_ino);
54+
// Prevent infinite recursion by tracking visited directories (dev,inode).
55+
if (!visited.insert(dir_id).second) {
56+
return absl::OkStatus();
57+
}
58+
return TraverseDirectory(path, callback, visited);
59+
}
60+
callback(path, path_stat);
61+
return absl::OkStatus();
62+
}
63+
64+
absl::Status TraverseDirectory(
65+
absl::string_view path,
66+
absl::FunctionRef<void(absl::string_view, const struct stat&)> callback,
67+
std::set<std::pair<dev_t, ino_t>>& visited) {
4068
DIR* dir = opendir(std::string(path).c_str());
4169
if (!dir) {
4270
return ErrnoStatus(absl::StrCat("could not open directory ", path), errno);
@@ -58,7 +86,7 @@ absl::Status TraverseDirectory(
5886
continue;
5987
}
6088
const std::string entry_path = absl::StrCat(path, "/", entry_name);
61-
status.Update(YieldFiles(entry_path, callback));
89+
status.Update(YieldFilesInternal(entry_path, callback, visited));
6290
}
6391
closedir(dir);
6492
return status;
@@ -69,15 +97,8 @@ absl::Status TraverseDirectory(
6997
absl::Status YieldFiles(
7098
absl::string_view path,
7199
absl::FunctionRef<void(absl::string_view, const struct stat&)> callback) {
72-
struct stat path_stat;
73-
if (stat(std::string(path).c_str(), &path_stat) < 0) {
74-
return ErrnoStatus(absl::StrCat("could not stat ", path), errno);
75-
}
76-
if (S_ISDIR(path_stat.st_mode)) {
77-
return TraverseDirectory(path, callback);
78-
}
79-
callback(path, path_stat);
80-
return absl::OkStatus();
100+
std::set<std::pair<dev_t, ino_t>> visited;
101+
return YieldFilesInternal(path, callback, visited);
81102
}
82103

83104
absl::Status SetFileContents(absl::string_view path,

fuzzing/replay/file_util_test.cc

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,11 @@
1616

1717
#include <sys/stat.h>
1818
#include <sys/types.h>
19+
#include <unistd.h>
1920

21+
#include <cerrno>
2022
#include <cstdlib>
23+
#include <cstring>
2124
#include <functional>
2225
#include <string>
2326
#include <vector>
@@ -113,6 +116,43 @@ TEST(YieldFilesTest, YieldsHiddenFilesAndDirs) {
113116
EXPECT_THAT(collected_paths, testing::SizeIs(2));
114117
}
115118

119+
TEST(YieldFilesTest, DoesNotRecurseThroughSymlinkLoop) {
120+
const std::string root_dir =
121+
absl::StrCat(getenv("TEST_TMPDIR"), "/symlink-loop-root");
122+
ASSERT_EQ(mkdir(root_dir.c_str(), 0755), 0);
123+
const std::string dir_a = absl::StrCat(root_dir, "/dirA");
124+
ASSERT_EQ(mkdir(dir_a.c_str(), 0755), 0);
125+
const std::string dir_b = absl::StrCat(root_dir, "/dirB");
126+
ASSERT_EQ(mkdir(dir_b.c_str(), 0755), 0);
127+
128+
// Normal files that must each be yielded exactly once.
129+
const std::string file_a = absl::StrCat(root_dir, "/a");
130+
const std::string file_b = absl::StrCat(root_dir, "/b");
131+
ASSERT_TRUE(SetFileContents(file_a, "foo").ok());
132+
ASSERT_TRUE(SetFileContents(file_b, "bar").ok());
133+
134+
// Build a symlink cycle: dirA/toB -> dirB and dirB/toA -> dirA. Without cycle
135+
// detection, traversal would recurse forever (dirA -> dirB -> dirA -> ...).
136+
const std::string link_to_b = absl::StrCat(dir_a, "/toB");
137+
if (symlink(dir_b.c_str(), link_to_b.c_str()) != 0) {
138+
GTEST_SKIP() << "symlink unsupported in this environment: "
139+
<< std::strerror(errno);
140+
}
141+
const std::string link_to_a = absl::StrCat(dir_b, "/toA");
142+
ASSERT_EQ(symlink(dir_a.c_str(), link_to_a.c_str()), 0);
143+
144+
std::vector<std::string> collected_paths;
145+
const absl::Status status =
146+
YieldFiles(root_dir, CollectPathsCallback(&collected_paths));
147+
EXPECT_TRUE(status.ok());
148+
// Only the two regular files are yielded; directories (including those reached
149+
// through the symlinks) never invoke the callback, and the cycle is visited
150+
// at most once, so the result is fully deterministic regardless of readdir
151+
// ordering.
152+
EXPECT_THAT(collected_paths,
153+
testing::UnorderedElementsAre(file_a, file_b));
154+
}
155+
116156
} // namespace
117157

118158
} // namespace fuzzing

0 commit comments

Comments
 (0)