From 2c446228f7e684e8b2e6e1efdc1eb0c748d8b692 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?th=C3=A9odore=20cl=C3=A9ment?=
 <213218350+thclemnt@users.noreply.github.com>
Date: Tue, 19 May 2026 13:40:46 +0200
Subject: [PATCH] fix(document): prevent filepath injection

---
 inc/document.class.php   | 64 ++++++++++++++++++++++++------
 inc/transfer.class.php   |  2 +
 tests/units/Document.php | 85 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 139 insertions(+), 12 deletions(-)
 create mode 100644 tests/units/Document.php

diff --git a/inc/document.class.php b/inc/document.class.php
index b62b2392e2..6a4b75ae21 100644
--- a/inc/document.class.php
+++ b/inc/document.class.php
@@ -49,6 +49,7 @@ class Document extends CommonDBTM {
    static $rightname                   = 'document';
    static $tag_prefix                  = '#';
    protected $usenotepad               = true;
+   private $file_copy_source           = null;
 
 
    static function getTypeName($nb = 0) {
@@ -56,6 +57,11 @@ static function getTypeName($nb = 0) {
    }
 
 
+   function setFileCopySource(Document $source) {
+      $this->file_copy_source = $source;
+   }
+
+
    /**
     * Check if given object can have Document
     *
@@ -194,15 +200,25 @@ function defineTabs($options = []) {
    function prepareInputForAdd($input) {
       global $CFG_GLPI;
 
-      // security (don't accept filename from $_REQUEST)
-      if (array_key_exists('filename', $_REQUEST)) {
-         unset($input['filename']);
-      }
+      $input = $this->filterFields($input);
+
+      // current_* fields are not necessary for a new item, but keeping
+      // request-provided values can lead to wrong file deletion in move*().
+      $input['current_filepath'] = '';
+      $input['current_filename'] = '';
 
       if ($uid = Session::getLoginUserID()) {
          $input["users_id"] = Session::getLoginUserID();
       }
 
+      if ($this->file_copy_source instanceof self) {
+         $input['filename'] = $this->file_copy_source->fields['filename'];
+         $input['filepath'] = $this->file_copy_source->fields['filepath'];
+         $input['sha1sum']  = $this->file_copy_source->fields['sha1sum'];
+         $input['mime']     = $this->file_copy_source->fields['mime'];
+         $this->file_copy_source = null;
+      }
+
       // Create a doc only selecting a file from a item form
       $create_from_item = false;
       if (isset($input["items_id"])
@@ -230,9 +246,6 @@ function prepareInputForAdd($input) {
       } else if (isset($input["upload_file"]) && !empty($input["upload_file"])) {
          // Move doc from upload dir
          $upload_ok = $this->moveUploadedDocument($input, $input["upload_file"]);
-      } else if (isset($input['filepath']) && file_exists(GLPI_DOC_DIR.'/'.$input['filepath'])) {
-         // Document is created using an existing document file
-         $upload_ok = true;
       }
 
       // Tag
@@ -256,6 +269,7 @@ function prepareInputForAdd($input) {
       }
 
       unset($input["upload_file"]);
+      unset($input['current_filepath']);
 
       // Don't add if no file
       if (isset($input["_only_if_upload_succeed"])
@@ -331,12 +345,16 @@ public function post_getFromDB() {
 
    function prepareInputForUpdate($input) {
 
-      // security (don't accept filename from $_REQUEST)
-      if (array_key_exists('filename', $_REQUEST)) {
-         unset($input['filename']);
-      }
+      $input = $this->filterFields($input);
+
+      if (isset($input['current_filepath'])
+          || isset($input["_filename"])
+          || isset($input["upload_file"])) {
+         // Always use the values stored in DB to prevent arbitrary file
+         // deletion or replacement using request-controlled paths.
+         $input['current_filepath'] = $this->fields['filepath'];
+         $input['current_filename'] = $this->fields['filename'];
 
-      if (isset($input['current_filepath'])) {
          if (isset($input["_filename"]) && !empty($input["_filename"]) == 1) {
             $this->moveDocument($input, stripslashes(array_shift($input["_filename"])));
          } else if (isset($input["upload_file"]) && !empty($input["upload_file"])) {
@@ -361,6 +379,28 @@ function prepareInputForUpdate($input) {
    }
 
 
+   /**
+    * Remove fields that must only be produced by server-side upload handling.
+    *
+    * @param array $input
+    *
+    * @return array
+   **/
+   private function filterFields(array $input): array {
+      if (array_key_exists('filename', $_REQUEST)) {
+         unset($input['filename']);
+      }
+
+      foreach (['filepath', 'sha1sum'] as $field) {
+         if (array_key_exists($field, $input)) {
+            unset($input[$field]);
+         }
+      }
+
+      return $input;
+   }
+
+
    /**
     * Print the document form
     *
diff --git a/inc/transfer.class.php b/inc/transfer.class.php
index 479141094d..5931a2f0af 100644
--- a/inc/transfer.class.php
+++ b/inc/transfer.class.php
@@ -2092,10 +2092,12 @@ function transferDocuments($itemtype, $ID, $newID) {
                   // not found : copy doc
                   if ($newdocID < 0) {
                      // 1 - create new item
+                     $source_document = clone $document;
                      unset($document->fields['id']);
                      $input    = $document->fields;
                      // Not set new entity Do by transferItem
                      unset($document->fields);
+                     $document->setFileCopySource($source_document);
                      $newdocID = $document->add(Toolbox::addslashes_deep($input));
                      // 2 - transfer as copy
                      $this->transferItem('Document', $item_ID, $newdocID);