Просмотр исходного кода

Refuse a proxied path that does not start with a slash (#2638)

In absolute-form the path follows the authority, so a path such as "@evil.example/x" made the proxy connect to another host and forward the origin's credentials. Both handle_request and open_stream_ now refuse such a path before anything is written.
metsw24-max 5 дней назад
Родитель
Сommit
276cf08e84
2 измененных файлов с 42 добавлено и 2 удалено
  1. 12 2
      httplib.h
  2. 30 0
      test/test.cc

+ 12 - 2
httplib.h

@@ -15942,8 +15942,11 @@ inline ClientImpl::StreamHandle ClientImpl::open_stream_(Request &req) {
   {
   {
     detail::BufferStream bstrm;
     detail::BufferStream bstrm;
 
 
-    if (detail::write_request_line(bstrm, req.method,
-                                   proxy_target_prefix() + req.path) < 0) {
+    // The path follows the authority in absolute-form, so one that does not
+    // begin with '/' (e.g. "@evil.example/x") would name another host.
+    auto prefix = proxy_target_prefix();
+    if ((!prefix.empty() && req.path[0] != '/') ||
+        detail::write_request_line(bstrm, req.method, prefix + req.path) < 0) {
       handle.error = Error::Write;
       handle.error = Error::Write;
       handle.response.reset();
       handle.response.reset();
       return handle;
       return handle;
@@ -16248,6 +16251,13 @@ inline bool ClientImpl::handle_request(Stream &strm, Request &req,
 
 
   auto prefix = proxy_target_prefix();
   auto prefix = proxy_target_prefix();
   if (!prefix.empty()) {
   if (!prefix.empty()) {
+    // The path follows the authority in absolute-form, so one that does not
+    // begin with '/' (e.g. "@evil.example/x") would name another host.
+    if (req.path[0] != '/') {
+      error = Error::Write;
+      return false;
+    }
+
     auto req2 = req;
     auto req2 = req;
     req2.path = prefix + req.path;
     req2.path = prefix + req.path;
     ret = process_request(strm, req2, res, close_connection, error);
     ret = process_request(strm, req2, res, close_connection, error);

+ 30 - 0
test/test.cc

@@ -30415,6 +30415,36 @@ TEST(StreamRequestHeadTest, GoesThroughAPlainHttpProxy) {
   EXPECT_EQ("Basic cHVzZXI6cHBhc3M=", proxy_authz);
   EXPECT_EQ("Basic cHVzZXI6cHBhc3M=", proxy_authz);
 }
 }
 
 
+// In absolute-form the path follows the authority, so a path that does not
+// begin with '/' would make the proxy connect to another host
+TEST(ProxyAbsoluteFormTest, PathMustNotExtendTheAuthority) {
+  std::vector<std::string> targets;
+  no_proxy_test::ScopedServer proxy;
+  proxy.svr().Get(".*", [&](const Request &req, Response &res) {
+    targets.push_back(req.target);
+    res.set_content("ok", "text/plain");
+  });
+  proxy.listen();
+
+  Client cli("origin.example", 8080);
+  cli.set_proxy("127.0.0.1", proxy.port());
+  cli.set_basic_auth("user", "pass");
+
+  for (const auto &path : {"@evil.example/x", ".evil.example/x", ":81/x"}) {
+    auto res = cli.Get(path);
+    EXPECT_FALSE(res) << path;
+
+    auto handle = cli.open_stream("GET", path);
+    EXPECT_FALSE(handle.is_valid()) << path;
+  }
+  EXPECT_TRUE(targets.empty());
+
+  auto res = cli.Get("/x");
+  ASSERT_TRUE(res);
+  ASSERT_EQ(1u, targets.size());
+  EXPECT_EQ("http://origin.example:8080/x", targets[0]);
+}
+
 TEST(NoProxyTest, ExactHostnameBypasses) {
 TEST(NoProxyTest, ExactHostnameBypasses) {
   ProxyAndTargetServers s;
   ProxyAndTargetServers s;
   auto cli = make_client("example.com", s);
   auto cli = make_client("example.com", s);