fix(builtin): escape directory and profiler names

src/brpc/builtin/common.cpp

@@ -25,6 +25,8 @@
 #include "butil/file_util.h"                 // butil::FilePath
 #include "butil/third_party/murmurhash3/murmurhash3.h"
 #include "butil/process_util.h"              // ReadCommandLine
+#include "brpc/controller.h"
+#include "brpc/grpc.h"
 #include "brpc/server.h"
 #include "brpc/reloadable_flags.h"
 #include "brpc/builtin/common.h"
@@ -122,27 +124,36 @@ void PrintRealDateTime(std::ostream& os, int64_t tm,
     }
 }
 
+static std::string EncodeUriPath(const char* path) {
+    std::string encoded;
+    PercentEncode(path, &encoded);
+    size_t pos = 0;
+    while ((pos = encoded.find("%2f", pos)) != std::string::npos) {
+        encoded.replace(pos, 3, "/");
+        ++pos;
+    }
+    return encoded;
+}
+
 std::ostream& operator<<(std::ostream& os, const PrintedAsDateTime& d) {
     PrintRealDateTime(os, d.realtime);
     return os;
 }
 
 std::ostream& operator<<(std::ostream& os, const Path& link) {
-    if (link.html_addr) {
-        if (link.html_addr != Path::LOCAL) {
-            os << "<a href=\"http://" << *link.html_addr << link.uri << "\">";
-        } else {
-            os << "<a href=\"" << link.uri << "\">";
-        }
+    const char* const text = (link.text ? link.text : link.uri);
+    if (!link.html_addr) {
+        os << text;
+        return os;
     }
-    if (link.text) {
-        os << link.text;
+
+    const std::string escaped_uri = WebEscape(EncodeUriPath(link.uri));
+    if (link.html_addr != Path::LOCAL) {
+        os << "<a href=\"http://" << *link.html_addr << escaped_uri << "\">";
     } else {
-        os << link.uri;
-    }
-    if (link.html_addr) {
-        os << "</a>";
+        os << "<a href=\"" << escaped_uri << "\">";
     }
+    os << WebEscape(text) << "</a>";
     return os;
 }
 

src/brpc/builtin/hotspots_service.cpp

@@ -24,8 +24,10 @@
 #include "butil/popen.h"                         // butil::read_command_output
 #include "butil/fd_guard.h"                      // butil::fd_guard
 #include "butil/iobuf_profiler.h"
+#include "butil/strings/string_util.h"
 #include "brpc/log.h"
 #include "brpc/controller.h"
+#include "brpc/grpc.h"
 #include "brpc/server.h"
 #include "brpc/reloadable_flags.h"
 #include "brpc/builtin/pprof_perl.h"
@@ -1063,6 +1065,9 @@ static void StartProfiling(ProfilingType type,
     for (butil::FilePath name = prof_enum.Next(); !name.empty();
          name = prof_enum.Next()) {
         // NOTE: name already includes dir.
+        if (!ValidProfilePath(name.value())) {
+            continue;
+        }
         if (past_profs.empty()) {
             past_profs.reserve(16);
         }
@@ -1095,11 +1100,11 @@ static void StartProfiling(ProfilingType type,
         "<select id='view_prof' onchange='onSelectProf()'>";
     os << "<option value=''>&lt;new profile&gt;</option>";
     for (size_t i = 0; i < past_profs.size(); ++i) {
-        os << "<option value='" << past_profs[i] << "' ";
+        os << "<option value='" << WebEscape(past_profs[i]) << "' ";
         if (view != nullptr && past_profs[i] == *view) {
             os << "selected";
         }
-        os << '>' << GetBaseName(&past_profs[i]);
+        os << '>' << WebEscape(GetBaseName(&past_profs[i]));
     }
     os << "</select>";
     os << "<div><pre style='display:inline'>Display: </pre>"
@@ -1120,11 +1125,11 @@ static void StartProfiling(ProfilingType type,
               "<select id='base_prof' onchange='onSelectProf()'>"
               "<option value=''>&lt;none&gt;</option>";
         for (size_t i = 0; i<past_profs.size(); ++i) {
-            os << "<option value='" << past_profs[i] << "' ";
+            os << "<option value='" << WebEscape(past_profs[i]) << "' ";
             if (base_name!=nullptr && past_profs[i]==*base_name) {
                 os << "selected";
             }
-            os << '>' << GetBaseName(&past_profs[i]);
+            os << '>' << WebEscape(GetBaseName(&past_profs[i]));
         }
         os << "</select></div>";
     }

test/brpc_builtin_service_unittest.cpp

@@ -22,16 +22,20 @@
 #include <sys/types.h>
 #include <sys/socket.h>
 #include <fstream>
+#include <sstream>
 #include <gtest/gtest.h>
 #include <gflags/gflags.h>
 #include <google/protobuf/descriptor.h>
 #include "gperftools_helper.h"
+#include "butil/file_util.h"
+#include "butil/files/scoped_temp_dir.h"
 #include "butil/time.h"
 #include "butil/macros.h"
 #include "brpc/socket.h"
 #include "brpc/server.h"
 #include "brpc/channel.h"
 #include "brpc/controller.h"
+#include "brpc/grpc.h"
 #include "brpc/span.h"
 #include "brpc/reloadable_flags.h"
 #include "brpc/builtin/version_service.h"
@@ -815,6 +819,76 @@ TEST_F(BuiltinServiceTest, hotspots_query_escaping) {
     }
 }
 
+TEST_F(BuiltinServiceTest, hotspots_profile_filename_escaping) {
+    GFLAGS_NAMESPACE::FlagSaver flag_saver;
+    butil::ScopedTempDir temp_dir;
+    ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+    brpc::FLAGS_rpc_profiling_dir = temp_dir.path().value();
+    const butil::FilePath profile_dir = temp_dir.path().Append(
+        brpc::GetProgramChecksum());
+    ASSERT_TRUE(butil::CreateDirectory(profile_dir));
+
+    const std::string filename = "profile'><img src=x onerror=alert(1)>.cpu";
+    const butil::FilePath profile_path = profile_dir.Append(filename);
+    ASSERT_EQ(0, butil::WriteFile(profile_path, "", 0));
+
+    ClosureChecker done;
+    brpc::Controller cntl;
+    brpc::HotspotsRequest req;
+    brpc::HotspotsResponse res;
+    brpc::HotspotsService service;
+    SetUpController(&cntl, true);
+    service.cpu(&cntl, &req, &res, &done);
+
+    const std::string& body = cntl.response_attachment().to_string();
+    EXPECT_EQ(std::string::npos, body.find(filename))
+        << "unescaped profile filename in html: " << body;
+    EXPECT_EQ(std::string::npos, body.find(brpc::WebEscape(filename)));
+
+}
+
+TEST_F(BuiltinServiceTest, hotspots_non_responsive_raw_profile_paths) {
+    GFLAGS_NAMESPACE::FlagSaver flag_saver;
+    butil::ScopedTempDir temp_dir;
+    ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+    brpc::FLAGS_rpc_profiling_dir = temp_dir.path().value();
+    const butil::FilePath profile_dir = temp_dir.path().Append(
+        brpc::GetProgramChecksum());
+    ASSERT_TRUE(butil::CreateDirectory(profile_dir));
+    const butil::FilePath view_path = profile_dir.Append("selected.cpu");
+    const butil::FilePath base_path = profile_dir.Append("base.cpu");
+    ASSERT_EQ(0, butil::WriteFile(view_path, "", 0));
+    ASSERT_EQ(0, butil::WriteFile(base_path, "", 0));
+
+    ClosureChecker done;
+    brpc::Controller cntl;
+    brpc::HotspotsRequest req;
+    brpc::HotspotsResponse res;
+    brpc::HotspotsService service;
+    SetUpController(&cntl, true);
+    cntl.http_request().uri().SetQuery("view", view_path.value());
+    cntl.http_request().uri().SetQuery("base", base_path.value());
+    service.cpu_non_responsive(&cntl, &req, &res, &done);
+    EXPECT_FALSE(cntl.Failed()) << cntl.ErrorText();
+}
+
+TEST_F(BuiltinServiceTest, path_html_escaping) {
+    const std::string payload = "\"'><script>alert(1)</script>&";
+    std::ostringstream html;
+    html << brpc::Path(payload.c_str(), brpc::Path::LOCAL, payload.c_str());
+    EXPECT_EQ(std::string::npos, html.str().find(payload));
+    EXPECT_NE(std::string::npos, html.str().find(brpc::WebEscape(payload)));
+
+    std::ostringstream reserved_html;
+    reserved_html << brpc::Path("/dir/a?x#fragment", brpc::Path::LOCAL);
+    EXPECT_NE(std::string::npos,
+              reserved_html.str().find("href=\"/dir/a%3fx%23fragment\""));
+
+    std::ostringstream text;
+    text << brpc::Path(payload.c_str(), nullptr, payload.c_str());
+    EXPECT_EQ(payload, text.str());
+}
+
 TEST_F(BuiltinServiceTest, rpcz_query_escaping) {
     // Restore -enable_rpcz on any exit of this test.
     GFLAGS_NAMESPACE::FlagSaver flag_saver;