# HG changeset patch
# User Alain Mazy <am@osimis.io>
# Date 1680081817 -7200
# Node ID f8f1c4a9a216733d936199a546245662fec0f0c1
# Parent  32df369198ac0ec80454f182ecc62484fe07b721
New configuration option 'RestApiWriteToFileSystemEnabled'

diff -r 32df369198ac -r f8f1c4a9a216 NEWS
--- a/NEWS	Tue Mar 28 10:48:13 2023 +0200
+++ b/NEWS	Wed Mar 29 11:23:37 2023 +0200
@@ -5,6 +5,8 @@
 -----------
 
 * Enforce the existence of the patient/study/instance while creating its archive
+* Security: New configuration option "RestApiWriteToFileSystemEnabled" to allow 
+  "/instances/../export" that is now disabled by default.
 
 Bug Fixes
 ---------
diff -r 32df369198ac -r f8f1c4a9a216 OrthancServer/Resources/Configuration.json
--- a/OrthancServer/Resources/Configuration.json	Tue Mar 28 10:48:13 2023 +0200
+++ b/OrthancServer/Resources/Configuration.json	Wed Mar 29 11:23:37 2023 +0200
@@ -766,6 +766,11 @@
   // with Orthanc 1.5.8, this URI is disabled by default for security.
   "ExecuteLuaEnabled" : false,
 
+  // Whether the Rest API can write to the filesystem (e.g. in 
+  // /instances/../export route). Starting with Orthanc 1.12.0, 
+  // this URI is disabled by default for security.
+  "RestApiWriteToFileSystemEnabled": false,
+
   // Set the timeout while serving HTTP requests by the embedded Web
   // server, in seconds. This corresponds to option
   // "request_timeout_ms" of Mongoose/Civetweb. It will set the socket
diff -r 32df369198ac -r f8f1c4a9a216 OrthancServer/Sources/OrthancRestApi/OrthancRestResources.cpp
--- a/OrthancServer/Sources/OrthancRestApi/OrthancRestResources.cpp	Tue Mar 28 10:48:13 2023 +0200
+++ b/OrthancServer/Sources/OrthancRestApi/OrthancRestResources.cpp	Wed Mar 29 11:23:37 2023 +0200
@@ -428,7 +428,12 @@
       call.GetDocumentation()
         .SetTag("Instances")
         .SetSummary("Write DICOM onto filesystem")
-        .SetDescription("Write the DICOM file onto the filesystem where Orthanc is running")
+        .SetDescription("Write the DICOM file onto the filesystem where Orthanc is running.  This is insecure for "
+                        "Orthanc servers that are remotely accessible since one could overwrite any system file.  "
+                        "Since Orthanc 1.12.0, this route is disabled by default and can be enabled thanks to "
+                        "the `RestApiWriteToFileSystemEnabled` configuration.")
+        .AddRequestType(MimeType_PlainText, "The Lua script to be executed")
+
         .SetUriArgument("id", "Orthanc identifier of the DICOM instance of interest")
         .AddRequestType(MimeType_PlainText, "Target path on the filesystem");
       return;
@@ -436,6 +441,14 @@
 
     ServerContext& context = OrthancRestApi::GetContext(call);
 
+    if (!context.IsRestApiWriteToFileSystemEnabled())
+    {
+      LOG(ERROR) << "The URI /instances/../export is disallowed for security, "
+                 << "check your configuration option `RestApiWriteToFileSystemEnabled`";
+      call.GetOutput().SignalError(HttpStatus_403_Forbidden);
+      return;
+    }
+
     std::string publicId = call.GetUriComponent("id", "");
 
     std::string dicom;
diff -r 32df369198ac -r f8f1c4a9a216 OrthancServer/Sources/OrthancRestApi/OrthancRestSystem.cpp
--- a/OrthancServer/Sources/OrthancRestApi/OrthancRestSystem.cpp	Tue Mar 28 10:48:13 2023 +0200
+++ b/OrthancServer/Sources/OrthancRestApi/OrthancRestSystem.cpp	Wed Mar 29 11:23:37 2023 +0200
@@ -259,7 +259,8 @@
         .SetTag("System")
         .SetSummary("Execute Lua script")
         .SetDescription("Execute the provided Lua script by the Orthanc server. This is very insecure for "
-                        "Orthanc servers that are remotely accessible, cf. configuration option `ExecuteLuaEnabled`")
+                        "Orthanc servers that are remotely accessible.  Since Orthanc 1.5.8, this route "
+                        "is disabled by default and can be enabled thanks to the `ExecuteLuaEnabled` configuration.")
         .AddRequestType(MimeType_PlainText, "The Lua script to be executed")
         .AddAnswerType(MimeType_PlainText, "Output of the Lua script");
       return;
@@ -270,7 +271,7 @@
     if (!context.IsExecuteLuaEnabled())
     {
       LOG(ERROR) << "The URI /tools/execute-script is disallowed for security, "
-                 << "check your configuration file";
+                 << "check your configuration option `ExecuteLuaEnabled`";
       call.GetOutput().SignalError(HttpStatus_403_Forbidden);
       return;
     }
diff -r 32df369198ac -r f8f1c4a9a216 OrthancServer/Sources/ServerContext.cpp
--- a/OrthancServer/Sources/ServerContext.cpp	Tue Mar 28 10:48:13 2023 +0200
+++ b/OrthancServer/Sources/ServerContext.cpp	Wed Mar 29 11:23:37 2023 +0200
@@ -327,6 +327,7 @@
     metricsRegistry_(new MetricsRegistry),
     isHttpServerSecure_(true),
     isExecuteLuaEnabled_(false),
+    isRestApiWriteToFileSystemEnabled_(false),
     overwriteInstances_(false),
     dcmtkTranscoder_(new DcmtkTranscoder),
     isIngestTranscoding_(false),
diff -r 32df369198ac -r f8f1c4a9a216 OrthancServer/Sources/ServerContext.h
--- a/OrthancServer/Sources/ServerContext.h	Tue Mar 28 10:48:13 2023 +0200
+++ b/OrthancServer/Sources/ServerContext.h	Wed Mar 29 11:23:37 2023 +0200
@@ -248,6 +248,7 @@
     std::unique_ptr<MetricsRegistry>  metricsRegistry_;
     bool isHttpServerSecure_;
     bool isExecuteLuaEnabled_;
+    bool isRestApiWriteToFileSystemEnabled_;
     bool overwriteInstances_;
 
     std::unique_ptr<StorageCommitmentReports>  storageCommitmentReports_;
@@ -487,6 +488,16 @@
       return isExecuteLuaEnabled_;
     }
 
+    void SetRestApiWriteToFileSystemEnabled(bool enabled)
+    {
+      isRestApiWriteToFileSystemEnabled_ = enabled;
+    }
+
+    bool IsRestApiWriteToFileSystemEnabled() const
+    {
+      return isRestApiWriteToFileSystemEnabled_;
+    }
+
     void SetOverwriteInstances(bool overwrite)
     {
       overwriteInstances_ = overwrite;
diff -r 32df369198ac -r f8f1c4a9a216 OrthancServer/Sources/main.cpp
--- a/OrthancServer/Sources/main.cpp	Tue Mar 28 10:48:13 2023 +0200
+++ b/OrthancServer/Sources/main.cpp	Wed Mar 29 11:23:37 2023 +0200
@@ -1172,6 +1172,18 @@
         LOG(WARNING) << "Remote LUA script execution is disabled";
       }
 
+      if (lock.GetConfiguration().GetBooleanParameter("RestApiWriteToFileSystemEnabled", false))
+      {
+        context.SetRestApiWriteToFileSystemEnabled(true);
+        LOG(WARNING) << "====> Your Rest API can write to the FileSystem.  Review your configuration option \"RestApiWriteToFileSystemEnabled\". "
+                     << "Your setup is POSSIBLY INSECURE <====";
+      }
+      else
+      {
+        context.SetRestApiWriteToFileSystemEnabled(false);
+        LOG(WARNING) << "Rest API can not write to the file system.";
+      }
+
       if (lock.GetConfiguration().GetBooleanParameter("WebDavEnabled", true))
       {
         const bool allowDelete = lock.GetConfiguration().GetBooleanParameter("WebDavDeleteAllowed", false);