Skip to content

Commit 8c254a5

Browse files
BillyONealBillyONeal
authored
Telemetry: defend against parent process loops (#1101)
1 parent 133f8fc commit 8c254a5

File tree

3 files changed

+99
-42
lines changed

3 files changed

+99
-42
lines changed

‎include/vcpkg/base/system.process.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#pragma once
22

3+
#include <vcpkg/base/fwd/files.h>
34
#include <vcpkg/base/fwd/optional.h>
45
#include <vcpkg/base/fwd/system.process.h>
56

@@ -167,7 +168,7 @@ namespace vcpkg
167168
std::string executable_name;
168169
};
169170

170-
Optional<ProcessStat> try_parse_process_stat_file(StringView text, StringView origin);
171+
Optional<ProcessStat> try_parse_process_stat_file(const FileContents& contents);
171172
void get_parent_process_list(std::vector<std::string>& ret);
172173

173174
bool succeeded(const ExpectedL<int>& maybe_exit) noexcept;

‎src/vcpkg-test/cgroup-parser.cpp‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#include <catch2/catch.hpp>
22

3+
#include <vcpkg/base/files.h>
34
#include <vcpkg/base/optional.h>
45
#include <vcpkg/base/stringview.h>
56
#include <vcpkg/base/system.process.h>
@@ -74,7 +75,7 @@ TEST_CASE ("parse proc/pid/stat file", "[cgroup-parser]")
7475
std::string contents =
7576
R"(4281 (cpptools-srv) S 4099 1676 1676 0 -1 1077936384 51165 303 472 0 81 25 0 0 20 0 10 0 829158 4924583936 39830 18446744073709551615 4194304 14147733 140725993620736 0 0 0 0 16781312 16386 0 0 0 17 1 0 0 5 0 0 16247120 16519160 29999104 140725993622792 140725993622920 140725993622920 140725993627556 0)";
7677

77-
auto maybe_stat = try_parse_process_stat_file(contents, "test");
78+
auto maybe_stat = try_parse_process_stat_file({contents, "test"});
7879
REQUIRE(maybe_stat.has_value());
7980
auto stat = maybe_stat.value_or_exit(VCPKG_LINE_INFO);
8081
CHECK(stat.ppid == 4099);
@@ -86,7 +87,7 @@ TEST_CASE ("parse proc/pid/stat file", "[cgroup-parser]")
8687
std::string contents =
8788
R"(4281 () S 4099 1676 1676 0 -1 1077936384 51165 303 472 0 81 25 0 0 20 0 10 0 829158 4924583936 39830 18446744073709551615 4194304 14147733 140725993620736 0 0 0 0 16781312 16386 0 0 0 17 1 0 0 5 0 0 16247120 16519160 29999104 140725993622792 140725993622920 140725993622920 140725993627556 0)";
8889

89-
auto maybe_stat = try_parse_process_stat_file(contents, "test");
90+
auto maybe_stat = try_parse_process_stat_file({contents, "test"});
9091
REQUIRE(maybe_stat.has_value());
9192
auto stat = maybe_stat.value_or_exit(VCPKG_LINE_INFO);
9293
CHECK(stat.ppid == 4099);
@@ -98,7 +99,7 @@ TEST_CASE ("parse proc/pid/stat file", "[cgroup-parser]")
9899
std::string contents =
99100
R"(4281 (<(' '<)(> ' ')>) S 4099 1676 1676 0 -1 1077936384 51165 303 472 0 81 25 0 0 20 0 10 0 829158 4924583936 39830 18446744073709551615 4194304 14147733 140725993620736 0 0 0 0 16781312 16386 0 0 0 17 1 0 0 5 0 0 16247120 16519160 29999104 140725993622792 140725993622920 140725993622920 140725993627556 0)";
100101

101-
auto maybe_stat = try_parse_process_stat_file(contents, "test");
102+
auto maybe_stat = try_parse_process_stat_file({contents, "test"});
102103
REQUIRE(maybe_stat.has_value());
103104
auto stat = maybe_stat.value_or_exit(VCPKG_LINE_INFO);
104105
CHECK(stat.ppid == 4099);
@@ -110,7 +111,7 @@ TEST_CASE ("parse proc/pid/stat file", "[cgroup-parser]")
110111
std::string contents =
111112
R"(4281 (0123456789abcdef) S 4099 1676 1676 0 -1 1077936384 51165 303 472 0 81 25 0 0 20 0 10 0 829158 4924583936 39830 18446744073709551615 4194304 14147733 140725993620736 0 0 0 0 16781312 16386 0 0 0 17 1 0 0 5 0 0 16247120 16519160 29999104 140725993622792 140725993622920 140725993622920 140725993627556 0)";
112113

113-
auto maybe_stat = try_parse_process_stat_file(contents, "test");
114+
auto maybe_stat = try_parse_process_stat_file({contents, "test"});
114115
REQUIRE(maybe_stat.has_value());
115116
auto stat = maybe_stat.value_or_exit(VCPKG_LINE_INFO);
116117
CHECK(stat.ppid == 4099);
@@ -122,7 +123,7 @@ TEST_CASE ("parse proc/pid/stat file", "[cgroup-parser]")
122123
std::string contents =
123124
R"(4281 (()()()()()()()()) S 4099 1676 1676 0 -1 1077936384 51165 303 472 0 81 25 0 0 20 0 10 0 829158 4924583936 39830 18446744073709551615 4194304 14147733 140725993620736 0 0 0 0 16781312 16386 0 0 0 17 1 0 0 5 0 0 16247120 16519160 29999104 140725993622792 140725993622920 140725993622920 140725993627556 0)";
124125

125-
auto maybe_stat = try_parse_process_stat_file(contents, "test");
126+
auto maybe_stat = try_parse_process_stat_file({contents, "test"});
126127
REQUIRE(maybe_stat.has_value());
127128
auto stat = maybe_stat.value_or_exit(VCPKG_LINE_INFO);
128129
CHECK(stat.ppid == 4099);
@@ -134,7 +135,7 @@ TEST_CASE ("parse proc/pid/stat file", "[cgroup-parser]")
134135
std::string contents =
135136
R"(4281 (0123456789abcdefg) S 4099 1676 1676 0 -1 1077936384 51165 303 472 0 81 25 0 0 20 0 10 0 829158 4924583936 39830 18446744073709551615 4194304 14147733 140725993620736 0 0 0 0 16781312 16386 0 0 0 17 1 0 0 5 0 0 16247120 16519160 29999104 140725993622792 140725993622920 140725993622920 140725993627556 0)";
136137

137-
auto maybe_stat = try_parse_process_stat_file(contents, "test");
138+
auto maybe_stat = try_parse_process_stat_file({contents, "test"});
138139
REQUIRE(!maybe_stat.has_value());
139140
}
140-
}
141+
}

‎src/vcpkg/base/system.process.cpp‎

Lines changed: 89 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
#include <ctime>
1414
#include <future>
15+
#include <set>
1516

1617
#if defined(__APPLE__)
1718
extern char** environ;
@@ -249,9 +250,9 @@ namespace vcpkg
249250
#endif
250251
}
251252

252-
Optional<ProcessStat> try_parse_process_stat_file(StringView text, StringView origin)
253+
Optional<ProcessStat> try_parse_process_stat_file(const FileContents& contents)
253254
{
254-
ParserBase p(text, origin);
255+
ParserBase p(contents.content, contents.origin);
255256

256257
p.match_while(ParserBase::is_ascii_digit); // pid %d (ignored)
257258

@@ -293,61 +294,115 @@ namespace vcpkg
293294
}
294295
return nullopt;
295296
}
297+
} // namespace vcpkg
296298

299+
namespace
300+
{
301+
#if defined(_WIN32)
302+
struct ToolHelpProcessSnapshot
303+
{
304+
ToolHelpProcessSnapshot() noexcept : snapshot(CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0)) { }
305+
ToolHelpProcessSnapshot(const ToolHelpProcessSnapshot&) = delete;
306+
ToolHelpProcessSnapshot& operator=(const ToolHelpProcessSnapshot&) = delete;
307+
~ToolHelpProcessSnapshot()
308+
{
309+
if (snapshot != INVALID_HANDLE_VALUE)
310+
{
311+
CloseHandle(snapshot);
312+
}
313+
}
314+
explicit operator bool() const noexcept { return snapshot != INVALID_HANDLE_VALUE; }
315+
316+
BOOL Process32First(PPROCESSENTRY32W entry) const noexcept { return Process32FirstW(snapshot, entry); }
317+
BOOL Process32Next(PPROCESSENTRY32W entry) const noexcept { return Process32NextW(snapshot, entry); }
318+
319+
private:
320+
HANDLE snapshot;
321+
};
322+
#elif defined(__linux__)
323+
Optional<ProcessStat> try_get_process_stat_by_pid(int pid)
324+
{
325+
auto filepath = fmt::format("/proc/{}/stat", pid);
326+
auto maybe_contents = real_filesystem.try_read_contents(filepath);
327+
if (auto contents = maybe_contents.get())
328+
{
329+
return try_parse_process_stat_file(*contents);
330+
}
331+
332+
return nullopt;
333+
}
334+
#endif // ^^^ __linux__
335+
} // unnamed namespace
336+
337+
namespace vcpkg
338+
{
297339
void get_parent_process_list(std::vector<std::string>& ret)
298340
{
299341
ret.clear();
300342
#if defined(_WIN32)
301343
// Enumerate all processes in the system snapshot.
302-
auto snapshot = CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0);
303-
if (snapshot == INVALID_HANDLE_VALUE) return;
304-
305344
std::map<DWORD, DWORD> pid_ppid_map;
306345
std::map<DWORD, std::string> pid_exe_path_map;
346+
std::set<DWORD> seen_pids;
307347

308-
PROCESSENTRY32W entry;
309-
memset(&entry, 0, sizeof(entry));
348+
PROCESSENTRY32W entry{};
310349
entry.dwSize = sizeof(entry);
311-
if (Process32FirstW(snapshot, &entry))
312350
{
313-
do
351+
ToolHelpProcessSnapshot snapshot;
352+
if (!snapshot)
314353
{
315-
pid_ppid_map.emplace(entry.th32ProcessID, entry.th32ParentProcessID);
316-
pid_exe_path_map.emplace(entry.th32ProcessID, Strings::to_utf8(entry.szExeFile));
317-
} while (Process32NextW(snapshot, &entry));
318-
}
319-
CloseHandle(snapshot);
354+
return;
355+
}
356+
357+
if (snapshot.Process32First(&entry))
358+
{
359+
do
360+
{
361+
pid_ppid_map.emplace(entry.th32ProcessID, entry.th32ParentProcessID);
362+
pid_exe_path_map.emplace(entry.th32ProcessID, Strings::to_utf8(entry.szExeFile));
363+
} while (snapshot.Process32Next(&entry));
364+
}
365+
} // destroy snapshot
320366

321367
// Find hierarchy of current process
322-
auto it = pid_ppid_map.find(GetCurrentProcessId());
323-
if (it == pid_ppid_map.end()) return;
324-
while (true)
368+
369+
for (DWORD next_parent = GetCurrentProcessId();;)
325370
{
326-
it = pid_ppid_map.find(it->second);
327-
if (it == pid_ppid_map.end()) break;
371+
if (Util::Sets::contains(seen_pids, next_parent))
372+
{
373+
// parent graph loops, for example if a parent terminates and the PID is reused by a child launch
374+
break;
375+
}
376+
377+
seen_pids.insert(next_parent);
378+
auto it = pid_ppid_map.find(next_parent);
379+
if (it == pid_ppid_map.end())
380+
{
381+
break;
382+
}
383+
328384
ret.push_back(pid_exe_path_map[it->first]);
385+
next_parent = it->second;
329386
}
330387
#elif defined(__linux__)
331-
std::error_code ec;
332-
auto vcpkg_stat_filepath = fmt::format("/proc/{}/stat", getpid());
333-
auto vcpkg_stat_contents = real_filesystem.read_contents(vcpkg_stat_filepath, ec);
334-
if (ec) return;
335-
336-
auto maybe_vcpkg_stat = try_parse_process_stat_file(vcpkg_stat_contents, vcpkg_stat_filepath);
388+
std::set<int> seen_pids;
389+
auto maybe_vcpkg_stat = try_get_process_stat_by_pid(getpid());
337390
if (auto vcpkg_stat = maybe_vcpkg_stat.get())
338391
{
339-
auto pid = vcpkg_stat->ppid;
340-
while (pid != 0)
392+
for (auto next_parent = vcpkg_stat->ppid; next_parent != 0;)
341393
{
342-
auto stat_filepath = fmt::format("/proc/{}/stat", pid);
343-
auto contents = real_filesystem.read_contents(stat_filepath, ec);
344-
if (ec) break;
394+
if (Util::Sets::contains(seen_pids, next_parent))
395+
{
396+
// parent graph loops, for example if a parent terminates and the PID is reused by a child launch
397+
break;
398+
}
345399

346-
auto maybe_stat = try_parse_process_stat_file(contents, stat_filepath);
347-
if (auto stat = maybe_stat.get())
400+
seen_pids.insert(next_parent);
401+
auto maybe_next_parent_stat = try_get_process_stat_by_pid(next_parent);
402+
if (auto next_parent_stat = maybe_next_parent_stat.get())
348403
{
349-
ret.push_back(stat->executable_name);
350-
pid = stat->ppid;
404+
ret.push_back(next_parent_stat->executable_name);
405+
next_parent = next_parent_stat->ppid;
351406
}
352407
else
353408
{

0 commit comments

Comments
 (0)