This is an automated email from the ASF dual-hosted git repository.
wasphin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/brpc.git
The following commit(s) were added to refs/heads/master by this push:
new 7a2c1d5e validate query parameters echoed into builtin html pages
(#3589)
7a2c1d5e is described below
commit 7a2c1d5e04b99dca884f87a534e32a86db491ed5
Author: UB <[email protected]>
AuthorDate: Mon Oct 12 10:13:22 2026 +0530
validate query parameters echoed into builtin html pages (#3589)
/hotspots/* wrote view and base into its inline script without the
ValidProfilePath check that its *_non_responsive handler already applies, and
/rpcz printed the pointer of the time parameter instead of the escaped value.
---
src/brpc/builtin/hotspots_service.cpp | 9 +++++
src/brpc/builtin/rpcz_service.cpp | 3 +-
test/brpc_builtin_service_unittest.cpp | 63 ++++++++++++++++++++++++++++++++++
3 files changed, 74 insertions(+), 1 deletion(-)
diff --git a/src/brpc/builtin/hotspots_service.cpp
b/src/brpc/builtin/hotspots_service.cpp
index 06ca5b27..7ed42953 100644
--- a/src/brpc/builtin/hotspots_service.cpp
+++ b/src/brpc/builtin/hotspots_service.cpp
@@ -914,6 +914,15 @@ static void StartProfiling(ProfilingType type,
}
#endif
}
+ // `view' and `base' are written into the page below and into the url of
+ // the request that page issues to *_non_responsive, which applies this
+ // same check before handing them to pprof.
+ if (view != nullptr && !ValidProfilePath(*view)) {
+ return cntl->SetFailed(EINVAL, "Invalid query `view'");
+ }
+ if (base_name != nullptr && !ValidProfilePath(*base_name)) {
+ return cntl->SetFailed(EINVAL, "Invalid query `base'");
+ }
ProfilingClient profiling_client;
size_t nwaiters = 0;
diff --git a/src/brpc/builtin/rpcz_service.cpp
b/src/brpc/builtin/rpcz_service.cpp
index fac905a4..989def2d 100644
--- a/src/brpc/builtin/rpcz_service.cpp
+++ b/src/brpc/builtin/rpcz_service.cpp
@@ -606,7 +606,8 @@ void
RpczService::default_method(::google::protobuf::RpcController* cntl_base,
} else {
start_tm = ParseDateTime(*time_str);
if (start_tm < 0) {
- os << "Invalid " << TIME_STR << "=`" << time_str << '\''
+ os << "Invalid " << TIME_STR << "=`"
+ << (use_html ? WebEscape(*time_str) : *time_str) << '\''
<< (use_html ? "</body></html>" : "");
os.move_to(cntl->response_attachment());
return;
diff --git a/test/brpc_builtin_service_unittest.cpp
b/test/brpc_builtin_service_unittest.cpp
index 2389b99b..958e0666 100644
--- a/test/brpc_builtin_service_unittest.cpp
+++ b/test/brpc_builtin_service_unittest.cpp
@@ -47,6 +47,7 @@
#include "brpc/builtin/rpcz_service.h" // RpczService
#include "brpc/builtin/dir_service.h" // DirService
#include "brpc/builtin/pprof_service.h" // PProfService
+#include "brpc/builtin/hotspots_service.h" // HotspotsService
#include "brpc/builtin/bthreads_service.h" // BthreadsService
#include "brpc/builtin/ids_service.h" // IdsService
#include "brpc/builtin/sockets_service.h" // SocketsService
@@ -753,6 +754,68 @@ TEST_F(BuiltinServiceTest, flags_escaping) {
}
}
+TEST_F(BuiltinServiceTest, hotspots_query_escaping) {
+ // `view' and `base' are written into the inline script of the page and
+ // are forwarded to /hotspots/*_non_responsive, which rejects them with
+ // ValidProfilePath.
+ const std::string payload =
+ "\" + alert(1) + \"</script><script>alert(2)</script>";
+ brpc::HotspotsService service;
+ {
+ ClosureChecker done;
+ brpc::Controller cntl;
+ brpc::HotspotsRequest req;
+ brpc::HotspotsResponse res;
+ SetUpController(&cntl, true);
+ cntl.http_request().uri().SetQuery("view", payload);
+ service.cpu(&cntl, &req, &res, &done);
+ EXPECT_TRUE(cntl.Failed());
+ EXPECT_EQ(EINVAL, cntl.ErrorCode());
+ const std::string body = cntl.response_attachment().to_string();
+ EXPECT_EQ(std::string::npos, body.find(payload))
+ << "unescaped payload in html: " << body;
+ }
+ {
+ ClosureChecker done;
+ brpc::Controller cntl;
+ brpc::HotspotsRequest req;
+ brpc::HotspotsResponse res;
+ SetUpController(&cntl, true);
+ // A well-formed `view' so that `base' is reached.
+ cntl.http_request().uri().SetQuery(
+ "view", brpc::FLAGS_rpc_profiling_dir + "/0/20240101.000000.cpu");
+ cntl.http_request().uri().SetQuery("base", payload);
+ service.cpu(&cntl, &req, &res, &done);
+ EXPECT_TRUE(cntl.Failed());
+ EXPECT_EQ(EINVAL, cntl.ErrorCode());
+ const std::string body = cntl.response_attachment().to_string();
+ EXPECT_EQ(std::string::npos, body.find(payload))
+ << "unescaped payload in html: " << body;
+ }
+}
+
+TEST_F(BuiltinServiceTest, rpcz_query_escaping) {
+ // Restore -enable_rpcz on any exit of this test.
+ GFLAGS_NAMESPACE::FlagSaver flag_saver;
+ ASSERT_FALSE(GFLAGS_NAMESPACE::SetCommandLineOption(
+ "enable_rpcz", "true").empty());
+ const std::string payload = "<svg onload=alert(1)>&\"'";
+ const std::string escaped = brpc::WebEscape(payload);
+ ClosureChecker done;
+ brpc::Controller cntl;
+ brpc::RpczRequest req;
+ brpc::RpczResponse res;
+ brpc::RpczService service;
+ SetUpController(&cntl, true);
+ cntl.http_request().uri().SetQuery(brpc::TIME_STR, payload);
+ service.default_method(&cntl, &req, &res, &done);
+ EXPECT_FALSE(cntl.Failed());
+ const std::string body = cntl.response_attachment().to_string();
+ EXPECT_EQ(std::string::npos, body.find(payload))
+ << "unescaped payload in html: " << body;
+ CheckContent(cntl, escaped.c_str());
+}
+
TEST_F(BuiltinServiceTest, bad_method) {
TestBadMethod(false);
TestBadMethod(true);
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
