From 52b5a8ece787e4d0c6d5ae893819777fdfe4368a Mon Sep 17 00:00:00 2001
From: Laurent Destailleur <eldy@destailleur.fr>
Date: Sun, 21 May 2017 14:06:43 +0200
Subject: [PATCH] More powerfull dol_move function.
---
htdocs/api/class/api_documents.class.php | 26 +--
htdocs/core/class/antivir.class.php | 7 +
htdocs/core/lib/files.lib.php | 216 ++++++++++++++---------
test/phpunit/RestAPIDocumentTest.php | 2 +-
4 files changed, 160 insertions(+), 91 deletions(-)
diff --git a/htdocs/api/class/api_documents.class.php b/htdocs/api/class/api_documents.class.php
index 18ccee58b07..9794dad4a12 100644
--- a/htdocs/api/class/api_documents.class.php
+++ b/htdocs/api/class/api_documents.class.php
@@ -65,8 +65,8 @@ class Documents extends DolibarrApi
/**
* Push a file.
- * Test sample 1: { "filename": "mynewfile.txt", "modulepart": "facture", "ref": "FA1701-001", "subdir": "", "filecontent": "content text", "fileencoding": "" }.
- * Test sample 2: { "filename": "mynewfile.txt", "modulepart": "medias", "ref": "", "subdir": "mysubdir1/mysubdir2", "filecontent": "content text", "fileencoding": "" }.
+ * Test sample 1: { "filename": "mynewfile.txt", "modulepart": "facture", "ref": "FA1701-001", "subdir": "", "filecontent": "content text", "fileencoding": "", "overwriteifexists": "0" }.
+ * Test sample 2: { "filename": "mynewfile.txt", "modulepart": "medias", "ref": "", "subdir": "mysubdir1/mysubdir2", "filecontent": "content text", "fileencoding": "", "overwriteifexists": "0" }.
*
* @param string $filename Name of file to create ('FA1705-0123')
* @param string $modulepart Name of module or area concerned by file upload ('facture', ...)
@@ -74,10 +74,12 @@ class Documents extends DolibarrApi
* @param string $subdir Subdirectory (Only if ref not provided)
* @param string $filecontent File content (string with file content. An empty file will be created if this parameter is not provided)
* @param string $fileencoding File encoding (''=no encoding, 'base64'=Base 64)
+ * @param int $overwriteifexists Overwrite file if exists (1 by default)
* @return bool State of copy
* @throws RestException
*/
- public function post($filename, $modulepart, $ref='', $subdir='', $filecontent='', $fileencoding='') {
+ public function post($filename, $modulepart, $ref='', $subdir='', $filecontent='', $fileencoding='', $overwriteifexists=0)
+ {
global $db, $conf;
/*var_dump($modulepart);
@@ -138,30 +140,34 @@ class Documents extends DolibarrApi
$upload_dir = dol_sanitizePathName($upload_dir);
- $destfile = $upload_dir . '/' . $original_file;
-
+ $destfile = $upload_dir . '/' . $original_file;
+ $destfiletmp = DOL_DATA_ROOT.'/admin/temp/' . $original_file;
+ dol_delete_file($destfiletmp);
+
if (!dol_is_dir($upload_dir)) {
throw new RestException(401,'Directory not exists : '.$upload_dir);
}
- if (dol_is_file($destfile))
+ if (! $overwriteifexists && dol_is_file($destfile))
{
throw new RestException(500, "File with name '".$original_file."' already exists.");
}
- $fhandle = fopen($destfile, 'w');
+ $fhandle = @fopen($destfiletmp, 'w');
if ($fhandle)
{
$nbofbyteswrote = fwrite($fhandle, $newfilecontent);
fclose($fhandle);
- @chmod($destfile, octdec($conf->global->MAIN_UMASK));
+ @chmod($destfiletmp, octdec($conf->global->MAIN_UMASK));
}
else
{
- throw new RestException(500, 'Failed to open file for write');
+ throw new RestException(500, "Failed to open file '".$destfiletmp."' for write");
}
- return true;
+ $result = dol_move($destfiletmp, $destfile, 0, $overwriteifexists, 1);
+
+ return $result;
}
/**
diff --git a/htdocs/core/class/antivir.class.php b/htdocs/core/class/antivir.class.php
index d7f765fa510..357f184eea8 100644
--- a/htdocs/core/class/antivir.class.php
+++ b/htdocs/core/class/antivir.class.php
@@ -49,6 +49,7 @@ class AntiVir
/**
* Scan a file with antivirus.
* This function runs the command defined in setup. This antivirus command must return 0 if OK.
+ * Return also true (virus found) if file end with '.virus' (so we can make test safely).
*
* @param string $file File to scan
* @return int <0 if KO (-98 if error, -99 if virus), 0 if OK
@@ -59,6 +60,12 @@ class AntiVir
$return = 0;
+ if (preg_match('/\.virus$/i', $file))
+ {
+ $this->errors='File has an extension saying file is a virus';
+ return -97;
+ }
+
$fullcommand=$this->getCliCommand($file);
//$fullcommand='"c:\Program Files (x86)\ClamWin\bin\clamscan.exe" --database="C:\Program Files (x86)\ClamWin\lib" "c:\temp\aaa.txt"';
$fullcommand.=' 2>&1'; // This is to get error output
diff --git a/htdocs/core/lib/files.lib.php b/htdocs/core/lib/files.lib.php
index 437811d8e98..008f4c15e3c 100644
--- a/htdocs/core/lib/files.lib.php
+++ b/htdocs/core/lib/files.lib.php
@@ -648,15 +648,19 @@ function dolCopyDir($srcfile, $destfile, $newmask, $overwriteifexists)
/**
* Move a file into another name.
- * This function differs from dol_move_uploaded_file, because it can be called in any context.
+ * Note:
+ * - This function differs from dol_move_uploaded_file, because it can be called in any context.
+ * - Database of files is updated.
+ * - Test on antivirus is done only if param testvirus is provided and an antivirus was set.
*
* @param string $srcfile Source file (can't be a directory. use native php @rename() to move a directory)
* @param string $destfile Destination file (can't be a directory. use native php @rename() to move a directory)
* @param integer $newmask Mask in octal string for new file (0 by default means $conf->global->MAIN_UMASK)
* @param int $overwriteifexists Overwrite file if exists (1 by default)
* @return boolean True if OK, false if KO
+ * @see dol_move_uploaded_file
*/
-function dol_move($srcfile, $destfile, $newmask=0, $overwriteifexists=1)
+function dol_move($srcfile, $destfile, $newmask=0, $overwriteifexists=1, $testvirus=0)
{
global $user, $db, $conf;
$result=false;
@@ -676,6 +680,18 @@ function dol_move($srcfile, $destfile, $newmask=0, $overwriteifexists=1)
$newpathofsrcfile=dol_osencode($srcfile);
$newpathofdestfile=dol_osencode($destfile);
+ // Check virus
+ $testvirusarray=array();
+ if ($testvirus)
+ {
+ $testvirusarray=dolCheckVirus($newpathofsrcfile);
+ if (count($testvirusarray))
+ {
+ dol_syslog("files.lib.php::dol_move canceled because a virus was found into source file. we ignore the move request.", LOG_WARNING);
+ return false;
+ }
+ }
+
$result=@rename($newpathofsrcfile, $newpathofdestfile); // To see errors, remove @
if (! $result)
{
@@ -703,9 +719,17 @@ function dol_move($srcfile, $destfile, $newmask=0, $overwriteifexists=1)
dol_syslog("Try to rename also entries in database for full relative path before = ".$rel_filetorenamebefore." after = ".$rel_filetorenameafter, LOG_DEBUG);
include_once DOL_DOCUMENT_ROOT.'/ecm/class/ecmfiles.class.php';
+
+ $ecmfiletarget=new EcmFiles($db);
+ $resultecmtarget = $ecmfiletarget->fetch(0, '', $rel_filetorenameafter);
+ if ($resultecmtarget > 0) // An entry for target name already exists for target, we delete it, a new one will be created.
+ {
+ $ecmfiletarget->delete($user);
+ }
+
$ecmfile=new EcmFiles($db);
- $result = $ecmfile->fetch(0, '', $rel_filetorenamebefore);
- if ($result > 0) // If found
+ $resultecm = $ecmfile->fetch(0, '', $rel_filetorenamebefore);
+ if ($resultecm > 0) // If an entry was found for src file, we use it to move entry
{
$filename = basename($rel_filetorenameafter);
$rel_dir = dirname($rel_filetorenameafter);
@@ -714,9 +738,9 @@ function dol_move($srcfile, $destfile, $newmask=0, $overwriteifexists=1)
$ecmfile->filepath = $rel_dir;
$ecmfile->filename = $filename;
- $result = $ecmfile->update($user);
+ $resultecm = $ecmfile->update($user);
}
- elseif ($result == 0) // If not found
+ elseif ($resultecm == 0) // If no entry were found for src files, create/update target file
{
$filename = basename($rel_filetorenameafter);
$rel_dir = dirname($rel_filetorenameafter);
@@ -730,16 +754,19 @@ function dol_move($srcfile, $destfile, $newmask=0, $overwriteifexists=1)
$ecmfile->gen_or_uploaded = 'unknown';
$ecmfile->description = ''; // indexed content
$ecmfile->keyword = ''; // keyword content
- $result = $ecmfile->create($user);
- if ($result < 0)
+ $resultecm = $ecmfile->create($user);
+ if ($resultecm < 0)
{
setEventMessages($ecmfile->error, $ecmfile->errors, 'warnings');
- }
+ }
}
- elseif ($result < 0)
+ elseif ($resultecm < 0)
{
setEventMessages($ecmfile->error, $ecmfile->errors, 'warnings');
}
+
+ if ($resultecm > 0) $result=true;
+ else $result = false;
}
}
@@ -769,10 +796,41 @@ function dol_unescapefile($filename)
return trim(basename($filename), ".\x00..\x20");
}
+
+/**
+ * Check virus into a file
+ *
+ * @param string $src_file Source file to check
+ * @return array Array of errors or empty array if not virus found
+ */
+function dolCheckVirus($src_file)
+{
+ global $conf;
+
+ if (! empty($conf->global->MAIN_ANTIVIRUS_COMMAND))
+ {
+ if (! class_exists('AntiVir')) {
+ require_once DOL_DOCUMENT_ROOT.'/core/class/antivir.class.php';
+ }
+ $antivir=new AntiVir($db);
+ $result = $antivir->dol_avscan_file($src_file);
+ if ($result < 0) // If virus or error, we stop here
+ {
+ $reterrors=$antivir->errors;
+ return $reterrors;
+ }
+ }
+ return array();
+}
+
+
/**
* Make control on an uploaded file from an GUI page and move it to final destination.
* If there is errors (virus found, antivir in error, bad filename), file is not moved.
- * Note: This function can be used only into a HTML page context. Use dol_move if you are outside.
+ * Note:
+ * - This function can be used only into a HTML page context. Use dol_move if you are outside.
+ * - Database of files is not updated.
+ * - Test on antivirus is always done (if antivirus set).
*
* @param string $src_file Source full path filename ($_FILES['field']['tmp_name'])
* @param string $dest_file Target full path filename ($_FILES['field']['name'])
@@ -794,80 +852,78 @@ function dol_move_uploaded_file($src_file, $dest_file, $allowoverwrite, $disable
if (empty($nohook))
{
- // If an upload error has been reported
- if ($uploaderrorcode)
- {
- switch($uploaderrorcode)
- {
- case UPLOAD_ERR_INI_SIZE: // 1
- return 'ErrorFileSizeTooLarge';
- break;
- case UPLOAD_ERR_FORM_SIZE: // 2
- return 'ErrorFileSizeTooLarge';
- break;
- case UPLOAD_ERR_PARTIAL: // 3
- return 'ErrorPartialFile';
- break;
- case UPLOAD_ERR_NO_TMP_DIR: //
- return 'ErrorNoTmpDir';
- break;
- case UPLOAD_ERR_CANT_WRITE:
- return 'ErrorFailedToWriteInDir';
- break;
- case UPLOAD_ERR_EXTENSION:
- return 'ErrorUploadBlockedByAddon';
- break;
- default:
- break;
- }
- }
-
- // If we need to make a virus scan
- if (empty($disablevirusscan) && file_exists($src_file) && ! empty($conf->global->MAIN_ANTIVIRUS_COMMAND))
- {
- if (! class_exists('AntiVir')) {
- require_once DOL_DOCUMENT_ROOT.'/core/class/antivir.class.php';
- }
- $antivir=new AntiVir($db);
- $result = $antivir->dol_avscan_file($src_file);
- if ($result < 0) // If virus or error, we stop here
- {
- $reterrors=$antivir->errors;
- dol_syslog('Files.lib::dol_move_uploaded_file File "'.$src_file.'" (target name "'.$dest_file.'") KO with antivirus: result='.$result.' errors='.join(',',$antivir->errors), LOG_WARNING);
- return 'ErrorFileIsInfectedWithAVirus: '.join(',',$reterrors);
- }
- }
-
- // Security:
- // Disallow file with some extensions. We renamed them.
- // Car si on a mis le rep documents dans un rep de la racine web (pas bien), cela permet d'executer du code a la demande.
- if (preg_match('/\.htm|\.html|\.php|\.pl|\.cgi$/i',$dest_file) && empty($conf->global->MAIN_DOCUMENT_IS_OUTSIDE_WEBROOT_SO_NOEXE_NOT_REQUIRED))
- {
- $file_name.= '.noexe';
- }
-
- // Security:
- // We refuse cache files/dirs, upload using .. and pipes into filenames.
- if (preg_match('/^\./',$src_file) || preg_match('/\.\./',$src_file) || preg_match('/[<>|]/',$src_file))
- {
- dol_syslog("Refused to deliver file ".$src_file, LOG_WARNING);
- return -1;
- }
-
- // Security:
- // On interdit fichiers caches, remontees de repertoire ainsi que les pipe dans les noms de fichiers.
- if (preg_match('/^\./',$dest_file) || preg_match('/\.\./',$dest_file) || preg_match('/[<>|]/',$dest_file))
- {
- dol_syslog("Refused to deliver file ".$dest_file, LOG_WARNING);
- return -2;
- }
-
$reshook=$hookmanager->initHooks(array('fileslib'));
$parameters=array('dest_file' => $dest_file, 'src_file' => $src_file, 'file_name' => $file_name, 'varfiles' => $varfiles, 'allowoverwrite' => $allowoverwrite);
$reshook=$hookmanager->executeHooks('moveUploadedFile', $parameters, $object);
}
-
+
+ if (empty($reshook))
+ {
+ // If an upload error has been reported
+ if ($uploaderrorcode)
+ {
+ switch($uploaderrorcode)
+ {
+ case UPLOAD_ERR_INI_SIZE: // 1
+ return 'ErrorFileSizeTooLarge';
+ break;
+ case UPLOAD_ERR_FORM_SIZE: // 2
+ return 'ErrorFileSizeTooLarge';
+ break;
+ case UPLOAD_ERR_PARTIAL: // 3
+ return 'ErrorPartialFile';
+ break;
+ case UPLOAD_ERR_NO_TMP_DIR: //
+ return 'ErrorNoTmpDir';
+ break;
+ case UPLOAD_ERR_CANT_WRITE:
+ return 'ErrorFailedToWriteInDir';
+ break;
+ case UPLOAD_ERR_EXTENSION:
+ return 'ErrorUploadBlockedByAddon';
+ break;
+ default:
+ break;
+ }
+ }
+
+ // If we need to make a virus scan
+ if (empty($disablevirusscan) && file_exists($src_file))
+ {
+ $checkvirusarray=dolCheckVirus($src_file);
+ if (count($checkvirusarray))
+ {
+ dol_syslog('Files.lib::dol_move_uploaded_file File "'.$src_file.'" (target name "'.$dest_file.'") KO with antivirus: result='.$result.' errors='.join(',',$checkvirusarray), LOG_WARNING);
+ return 'ErrorFileIsInfectedWithAVirus: '.join(',',$checkvirusarray);
+ }
+ }
+
+ // Security:
+ // Disallow file with some extensions. We renamed them.
+ // Car si on a mis le rep documents dans un rep de la racine web (pas bien), cela permet d'executer du code a la demande.
+ if (preg_match('/\.htm|\.html|\.php|\.pl|\.cgi$/i',$dest_file) && empty($conf->global->MAIN_DOCUMENT_IS_OUTSIDE_WEBROOT_SO_NOEXE_NOT_REQUIRED))
+ {
+ $file_name.= '.noexe';
+ }
+
+ // Security:
+ // We refuse cache files/dirs, upload using .. and pipes into filenames.
+ if (preg_match('/^\./',$src_file) || preg_match('/\.\./',$src_file) || preg_match('/[<>|]/',$src_file))
+ {
+ dol_syslog("Refused to deliver file ".$src_file, LOG_WARNING);
+ return -1;
+ }
+
+ // Security:
+ // On interdit fichiers caches, remontees de repertoire ainsi que les pipe dans les noms de fichiers.
+ if (preg_match('/^\./',$dest_file) || preg_match('/\.\./',$dest_file) || preg_match('/[<>|]/',$dest_file))
+ {
+ dol_syslog("Refused to deliver file ".$dest_file, LOG_WARNING);
+ return -2;
+ }
+ }
+
if ($reshook < 0) // At least one blocking error returned by one hook
{
$errmsg = join(',', $hookmanager->errors);
diff --git a/test/phpunit/RestAPIDocumentTest.php b/test/phpunit/RestAPIDocumentTest.php
index b180586aa91..b60a7e9e833 100644
--- a/test/phpunit/RestAPIDocumentTest.php
+++ b/test/phpunit/RestAPIDocumentTest.php
@@ -172,7 +172,7 @@ class RestAPIDocumentTest extends PHPUnit_Framework_TestCase
dol_mkdir(DOL_DATA_ROOT.'/medias/tmpphpunit/tmpphpunit2');
$data = array(
- 'filename'=>"mynewfilethatwillfails.txt",
+ 'filename'=>"mynewfile.txt",
'modulepart'=>"medias",
'ref'=>"",
'subdir'=>"tmpphpunit/tmpphpunit2",
--
GitLab