changeset 6557:da36804105e3

fix filename argument security issue
author Alain Mazy <am@orthanc.team>
date Mon, 12 Jan 2026 18:46:50 +0100
parents c3bc970cf169
children 72aca3bddaf7
files NEWS OrthancFramework/Sources/HttpServer/HttpOutput.cpp
diffstat 2 files changed, 14 insertions(+), 3 deletions(-) [+]
line wrap: on
line diff
--- a/NEWS	Fri Jan 09 12:11:35 2026 +0100
+++ b/NEWS	Mon Jan 12 18:46:50 2026 +0100
@@ -11,6 +11,9 @@
 * Added new metrics in "/tools/metrics-prometheus":
   - "orthanc_logged_errors_count" 
   - "orthanc_logged_warnings_count"
+* Fixed a security issue where one could hijack HTTP headers in the response
+  through the `filename` argument of "/.../file" or "/.../archive" routes.
+
 
 Maintenance
 -----------
--- a/OrthancFramework/Sources/HttpServer/HttpOutput.cpp	Fri Jan 09 12:11:35 2026 +0100
+++ b/OrthancFramework/Sources/HttpServer/HttpOutput.cpp	Mon Jan 12 18:46:50 2026 +0100
@@ -37,7 +37,7 @@
 #include <vector>
 #include <stdio.h>
 #include <boost/lexical_cast.hpp>
-
+#include <boost/regex.hpp>
 
 #if ORTHANC_ENABLE_CIVETWEB == 1
 #  if !defined(CIVETWEB_HAS_DISABLE_KEEP_ALIVE)
@@ -121,10 +121,18 @@
     return isContentCompressible_;
   }
 
+  static std::string SanitizeFileName(const char* filename)
+  {
+    const boost::regex pattern("[\r\n\"]");
+
+    return boost::regex_replace(std::string(filename), pattern, "");
+  }
+
   void HttpOutput::StateMachine::SetContentFilename(const char* filename)
   {
-    // TODO Escape double quotes
-    AddHeader("Content-Disposition", "filename=\"" + std::string(filename) + "\"");
+    std::string sanitized = SanitizeFileName(filename);  // since the filename might come from the API, we need to sanitize it to make 
+                                                         // sure it does not add extra headers in the response with e.g: 'filename=toto.dcm"\r\nSet-Cookie:evil=1"
+    AddHeader("Content-Disposition", "filename=\"" + sanitized + "\"");
   }
 
   void HttpOutput::StateMachine::SetCookie(const std::string& cookie,