Freeze bvar dump path flags and cap pprof duration by FLAGS_max_profiling_seconds

[chenBright]

What problem does this PR solve?

Issue Number: resolve

Problem Summary:

Two builtin services let whoever reaches them do more than the surrounding
configuration intends.

/flags can repoint the bvar dump files.

The bvar dump flags carry an accept-anything validator whose only job is to
wake the dumping thread:

// src/bvar/variable.cpp
static bool wakeup_dumping_thread(const char*, const std::string&) {
    pthread_cond_signal(&dump_cond);
    return true;
}

The dumping thread re-reads those flags every round and ends up in
FileDumper::dump_impl():

butil::CreateDirectoryAndGetError(dir, &error);   // recursive mkdir
_fp = fopen(_filename.c_str(), "w");              // truncate

So two requests against a server with default options:

GET /flags/bvar_dump_file?setvalue=/victim/dir/target
GET /flags/bvar_dump?setvalue=true

create a directory chain anywhere and truncate a file inside it, with the
privileges of the server. setvalue is a GET, so this is reachable from a
browser as well.

The primitive is bounded but real. bvar_dump_file and bvar_dump_tabs get a
.data suffix forced on them by FilePath::AddExtension(), and the bytes
written are bvar names and values, not attacker input. mbvar_dump_file is
used verbatim with no suffix. bvar_dump_tabs names files too: its tab names
go through the same AddExtension(), which appends them to the path with no
sanitization, so ../ in a tab name escapes the configured directory. The
recursive mkdir and the truncation are unrestricted in every case.

/pprof ignores FLAGS_max_profiling_seconds, which /hotspots enforces.

// src/brpc/builtin/pprof_service.cpp, ReadSeconds(), before this PR
    return seconds;                                        // no upper bound

// src/brpc/builtin/hotspots_service.cpp:233
    seconds = std::min(seconds, FLAGS_max_profiling_seconds);

?seconds=2147483647 reaches bthread_usleep(sleep_sec * 1000000L), roughly
68 years. The profilers are process wide -- gperftools' ProfilerStart and
bthread's g_cp are each a single global, so this is not merely one hung
RPC: it holds the only CPU or contention profiler for as long as it asked for,
and /hotspots/cpu and /hotspots/contention answer 503 the whole time. The
300 second limit was enforced on only one of the two paths to the same
resource, so the path without it decides the limit for both.

What is changed and the side effects?

Changed:

• Drop the validator registration for -bvar_dump_file, -bvar_dump_tabsand FLAGS_mbvar_dump_file`.
• Clamp ReadSeconds() to FLAGS_max_profiling_seconds.

Side effects:

• Performance effects:

• Breaking backward compatibility:

---

Check List:

• Please make sure your changes are compilable.
• When providing us with a new feature, it is best to add related tests.
• Please follow Contributor Covenant Code of Conduct.

[Copilot]

🟡 Changes recommended

The new /pprof duration clamp can produce misleading errors in edge cases (e.g., max_profiling_seconds=0), and the security-relevant behavior changes would benefit from targeted unit test coverage to prevent regressions.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR hardens brpc builtin endpoints by preventing runtime repointing of bvar dump output paths via /flags, and by ensuring /pprof respects the same profiling-duration cap as /hotspots.

Changes:

• Removed gflags validators for bvar_dump_file, bvar_dump_tabs, and mbvar_dump_file so /flags/...?...setvalue= can no longer modify these path-like flags at runtime.
• Added clamping in /pprof’s ReadSeconds() to FLAGS_max_profiling_seconds (consistent with /hotspots).
• Centralized max_profiling_seconds flag definition in builtin/common.{h,cpp} for shared use.

File summaries

File	Description
src/bvar/variable.cpp	Stops making dump path-related flags reloadable via /flags by removing their validator registration; updates flag descriptions accordingly.
src/brpc/builtin/pprof_service.cpp	Clamps requested profiling duration to FLAGS_max_profiling_seconds.
src/brpc/builtin/hotspots_service.cpp	Removes local definition/validation of max_profiling_seconds (now shared).
src/brpc/builtin/common.h	Declares max_profiling_seconds for builtin services.
src/brpc/builtin/common.cpp	Defines and validates max_profiling_seconds in a shared location.

Review details

• Files reviewed: 5/5 changed files
• Comments generated: 3
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.