3 Commits 4d7c9a788d ... f787f31b87

Autor SHA1 Mensaje Fecha
  yhirose f787f31b87 Implement symlink protection in static file server and add corresponding tests hace 2 semanas
  yhirose 43a54a3e3d Add tests for Unicode path component decoding in decode_path_component function hace 2 semanas
  yhirose 83e98a28dd Add filename sanitization function and tests to prevent path traversal vulnerabilities hace 2 semanas
Se han modificado 3 ficheros con 175 adiciones y 6 borrados
  1. 19 4
      README.md
  2. 77 2
      httplib.h
  3. 79 0
      test/test.cc

+ 19 - 4
README.md

@@ -350,6 +350,11 @@ The following are built-in mappings:
 > [!WARNING]
 > These static file server methods are not thread-safe.
 
+<!-- -->
+
+> [!NOTE]
+> On POSIX systems, the static file server rejects requests that resolve (via symlinks) to a path outside the mounted base directory. Ensure that the served directory has appropriate permissions, as managing access to the served directory is the application developer's responsibility.
+
 ### File request handler
 
 ```cpp
@@ -541,16 +546,16 @@ svr.Post("/multipart", [&](const Request& req, Response& res) {
     }
 
     // IMPORTANT: file.filename is an untrusted value from the client.
-    // Always extract only the basename to prevent path traversal attacks.
-    auto safe_name = std::filesystem::path(file.filename).filename();
-    if (safe_name.empty() || safe_name == "." || safe_name == "..") {
+    // Always sanitize to prevent path traversal attacks.
+    auto safe_name = httplib::sanitize_filename(file.filename);
+    if (safe_name.empty()) {
       res.status = StatusCode::BadRequest_400;
       res.set_content("Invalid filename", "text/plain");
       return;
     }
 
     // Save to disk
-    std::ofstream ofs(upload_dir / safe_name, std::ios::binary);
+    std::ofstream ofs(upload_dir + "/" + safe_name, std::ios::binary);
     ofs << file.content;
   }
 
@@ -586,6 +591,16 @@ svr.Post("/multipart", [&](const Request& req, Response& res) {
 });
 ```
 
+#### Filename Sanitization
+
+`file.filename` in multipart uploads is an untrusted value from the client. Always sanitize before using it in file paths:
+
+```cpp
+auto safe = httplib::sanitize_filename(file.filename);
+```
+
+This function strips path separators (`/`, `\`), null bytes, leading/trailing whitespace, and rejects `.` and `..`. Returns an empty string if the filename is unsafe.
+
 ### Receive content with a content receiver
 
 ```cpp

+ 77 - 2
httplib.h

@@ -1773,6 +1773,7 @@ private:
   struct MountPointEntry {
     std::string mount_point;
     std::string base_dir;
+    std::string resolved_base_dir;
     Headers headers;
   };
   std::vector<MountPointEntry> base_dirs_;
@@ -2915,6 +2916,8 @@ std::string encode_query_component(const std::string &component,
 std::string decode_query_component(const std::string &component,
                                    bool plus_as_space = true);
 
+std::string sanitize_filename(const std::string &filename);
+
 std::string append_query_params(const std::string &path, const Params &params);
 
 std::pair<std::string, std::string> make_range_header(const Ranges &ranges);
@@ -4834,6 +4837,30 @@ inline bool is_valid_path(const std::string &path) {
   return true;
 }
 
+inline bool canonicalize_path(const char *path, std::string &resolved) {
+#if defined(_WIN32)
+  char buf[_MAX_PATH];
+  if (_fullpath(buf, path, _MAX_PATH) == nullptr) { return false; }
+  resolved = buf;
+#else
+  char buf[PATH_MAX];
+  if (realpath(path, buf) == nullptr) { return false; }
+  resolved = buf;
+#endif
+  return true;
+}
+
+inline bool is_path_within_base(const std::string &resolved_path,
+                                const std::string &resolved_base) {
+#if defined(_WIN32)
+  return _strnicmp(resolved_path.c_str(), resolved_base.c_str(),
+                   resolved_base.size()) == 0;
+#else
+  return strncmp(resolved_path.c_str(), resolved_base.c_str(),
+                 resolved_base.size()) == 0;
+#endif
+}
+
 inline FileStat::FileStat(const std::string &path) {
 #if defined(_WIN32)
   auto wpath = u8string_to_wstring(path.c_str());
@@ -9290,7 +9317,8 @@ inline std::string decode_path_component(const std::string &component) {
         // Unicode %uXXXX encoding
         auto val = 0;
         if (detail::from_hex_to_i(component, i + 2, 4, val)) {
-          // 4 digits Unicode codes
+          // 4 digits Unicode codes: val is 0x0000-0xFFFF (from 4 hex digits),
+          // so to_utf8 writes at most 3 bytes. buff[4] is safe.
           char buff[4];
           size_t len = detail::to_utf8(val, buff);
           if (len > 0) { result.append(buff, len); }
@@ -9395,6 +9423,30 @@ inline std::string decode_query_component(const std::string &component,
   return result;
 }
 
+inline std::string sanitize_filename(const std::string &filename) {
+  // Extract basename: find the last path separator (/ or \)
+  auto pos = filename.find_last_of("/\\");
+  auto result =
+      (pos != std::string::npos) ? filename.substr(pos + 1) : filename;
+
+  // Strip null bytes
+  result.erase(std::remove(result.begin(), result.end(), '\0'), result.end());
+
+  // Trim whitespace
+  {
+    auto start = result.find_first_not_of(" \t");
+    auto end = result.find_last_not_of(" \t");
+    result = (start == std::string::npos)
+                 ? ""
+                 : result.substr(start, end - start + 1);
+  }
+
+  // Reject . and ..
+  if (result == "." || result == "..") { return ""; }
+
+  return result;
+}
+
 inline std::string append_query_params(const std::string &path,
                                        const Params &params) {
   std::string path_with_query = path;
@@ -10523,7 +10575,18 @@ inline bool Server::set_mount_point(const std::string &mount_point,
   if (stat.is_dir()) {
     std::string mnt = !mount_point.empty() ? mount_point : "/";
     if (!mnt.empty() && mnt[0] == '/') {
-      base_dirs_.push_back({std::move(mnt), dir, std::move(headers)});
+      std::string resolved_base;
+      if (detail::canonicalize_path(dir.c_str(), resolved_base)) {
+#if defined(_WIN32)
+        if (resolved_base.back() != '\\' && resolved_base.back() != '/') {
+          resolved_base += '\\';
+        }
+#else
+        if (resolved_base.back() != '/') { resolved_base += '/'; }
+#endif
+      }
+      base_dirs_.push_back(
+          {std::move(mnt), dir, std::move(resolved_base), std::move(headers)});
       return true;
     }
   }
@@ -11103,6 +11166,18 @@ inline bool Server::handle_file_request(Request &req, Response &res) {
         auto path = entry.base_dir + sub_path;
         if (path.back() == '/') { path += "index.html"; }
 
+        // Defense-in-depth: is_valid_path blocks ".." traversal in the URL,
+        // but symlinks/junctions can still escape the base directory.
+        if (!entry.resolved_base_dir.empty()) {
+          std::string resolved_path;
+          if (detail::canonicalize_path(path.c_str(), resolved_path) &&
+              !detail::is_path_within_base(resolved_path,
+                                           entry.resolved_base_dir)) {
+            res.status = StatusCode::Forbidden_403;
+            return true;
+          }
+        }
+
         detail::FileStat stat(path);
 
         if (stat.is_dir()) {

+ 79 - 0
test/test.cc

@@ -413,6 +413,38 @@ TEST(DecodePathTest, PercentCharacterNUL) {
   EXPECT_EQ(decode_path_component("x%00x"), expected);
 }
 
+TEST(DecodePathTest, UnicodeEncoding) {
+  // %u0041 = 'A' (1-byte UTF-8)
+  EXPECT_EQ("A", decode_path_component("%u0041"));
+  // %u00E9 = 'é' (2-byte UTF-8)
+  EXPECT_EQ(U8("é"), decode_path_component("%u00E9"));
+  // %u3042 = 'あ' (3-byte UTF-8)
+  EXPECT_EQ(U8("あ"), decode_path_component("%u3042"));
+  // %uFFFF = max 4-digit hex (3-byte UTF-8, must not overflow buff[4])
+  EXPECT_FALSE(decode_path_component("%uFFFF").empty());
+  // %uD800 = surrogate (invalid, silently dropped)
+  EXPECT_EQ("", decode_path_component("%uD800"));
+}
+
+TEST(SanitizeFilenameTest, VariousPatterns) {
+  // Path traversal
+  EXPECT_EQ("passwd", httplib::sanitize_filename("../../../etc/passwd"));
+  EXPECT_EQ("passwd", httplib::sanitize_filename("..\\..\\etc\\passwd"));
+  EXPECT_EQ("file.txt", httplib::sanitize_filename("path/to\\..\\file.txt"));
+  // Normal and edge cases
+  EXPECT_EQ("photo.jpg", httplib::sanitize_filename("photo.jpg"));
+  EXPECT_EQ("filename.txt",
+            httplib::sanitize_filename("/path/to/filename.txt"));
+  EXPECT_EQ(".gitignore", httplib::sanitize_filename(".gitignore"));
+  EXPECT_EQ("", httplib::sanitize_filename(".."));
+  EXPECT_EQ("", httplib::sanitize_filename(""));
+  // Null bytes stripped
+  EXPECT_EQ("safe.txt",
+            httplib::sanitize_filename(std::string("safe\0.txt", 9)));
+  // Whitespace-only rejected
+  EXPECT_EQ("", httplib::sanitize_filename("   "));
+}
+
 TEST(EncodeQueryParamTest, ParseUnescapedChararactersTest) {
   string unescapedCharacters = "-_.!~*'()";
 
@@ -17044,3 +17076,50 @@ TEST_F(WebSocketSSLIntegrationTest, TextEcho) {
   client.close();
 }
 #endif
+
+#if !defined(_WIN32)
+TEST(SymlinkTest, SymlinkEscapeFromBaseDirectory) {
+  auto secret_dir = std::string("./symlink_test_secret");
+  auto served_dir = std::string("./symlink_test_served");
+  auto secret_file = secret_dir + "/secret.txt";
+  auto symlink_path = served_dir + "/escape";
+
+  // Setup: create directories and files
+  mkdir(secret_dir.c_str(), 0755);
+  mkdir(served_dir.c_str(), 0755);
+
+  {
+    std::ofstream ofs(secret_file);
+    ofs << "SECRET_DATA";
+  }
+
+  // Create symlink using absolute path so it resolves correctly
+  char abs_secret[PATH_MAX];
+  ASSERT_NE(nullptr, realpath(secret_dir.c_str(), abs_secret));
+  ASSERT_EQ(0, symlink(abs_secret, symlink_path.c_str()));
+
+  auto se = detail::scope_exit([&] {
+    unlink(symlink_path.c_str());
+    unlink(secret_file.c_str());
+    rmdir(served_dir.c_str());
+    rmdir(secret_dir.c_str());
+  });
+
+  Server svr;
+  svr.set_mount_point("/", served_dir);
+
+  auto listen_thread = std::thread([&svr]() { svr.listen("localhost", PORT); });
+  auto se2 = detail::scope_exit([&] {
+    svr.stop();
+    listen_thread.join();
+  });
+  svr.wait_until_ready();
+
+  Client cli("localhost", PORT);
+
+  // Symlink pointing outside base dir should be blocked
+  auto res = cli.Get("/escape/secret.txt");
+  ASSERT_TRUE(res);
+  EXPECT_EQ(StatusCode::Forbidden_403, res->status);
+}
+#endif