Supporting each other
Community forums
|
Welcome,
Guest
|
Report any technical problems you discover and discuss solutions.
TOPIC:
v3: Enabling file upload/import checks - patches 8 years 8 months ago #3056
|
Hello,
Version 3.0 of Xerte introduced some code to perform checks on files. These were an antivirus check, a file extension check and a file MIME type check. (V3.0 also included an XML parser check for files, but this has not been changed at all.) However, I could not see how the code for the three checks worked since it only seemed to be called in one or two places, and I couldn't find how to cause that code to be executed. My own testing showed that imported and uploaded files were not being checked. I have now enabled the checks in more places, but it required a bit of a change to some of the code. In particular the checking code needed to handle the $_FILES array format correctly. It also needed to cater for a second simpler array format. To enable this the code now builds its own simple associative array, using the real and temporary file names, which is then more easily processed later on. I have also made the code structure similar in all three checks. Unfortunately the error reporting is not so good, and I have to some extent relied on anything failing being logged to the PHP log file. This does mean that an upload may throw up an error popup indicating a virus problem when in fact it might be that the file extension or MIME type are illegal. Unfortunately at that stage it is not easy to distinguish one check from another, so the message has to be vaguely generic. Any file failing a check will return a 'false' value, which in turn means that the following checks will not be run. This can mean that, for example, a file failing an extension check may subsequently still fail another check. Any file failing a check is deleted from the server. Projects are imported as zip files, and these are checked when all the relevant files have been extracted from the zip file (into the import directory). Any file failing a check causes all the files to be deleted. If multiple individual files are selected for uploading, then each file is checked as it is uploaded. If a file fails a check, then any previously uploaded files are left intact, but any subsequent uploads are aborted. For example, if 3 files are selected for uploading, and file 2 has a virus, then file 1 will be uploaded successfully, file 2 will fail the check and not be uploaded and the upload of file 3 is not attempted. It is possible to enable specific checks by installing the relevant software. If the software (or PHP function) is not present, then that particular check is not run. subsequent checks will still run though. For the antivirus check, the ClamAV 'clamscan' command needs to be present. For the file extension check the 'pathinfo' function must be present, and for the MIME type check the 'mime_content_type' function must be present. (For the XML parser check the simplexml extension must be present.) I have provided patches below for the file check changes. I have tested these uploading different files, and project zip files. I have also disabled different checks to ensure that the remaining ones run correctly. There are 7 files involved, but most of the changes are actually quite small. These are in no particular order: --- website_code/php/import/fileupload.php.orig 2015-07-29 23:17:24.000000000 +0100
+++ website_code/php/import/fileupload.php 2015-08-20 17:23:08.531066252 +0100
@@ -18,10 +18,11 @@
* limitations under the License.
*/
require_once "../../../config.php";
+require_once "../../../plugins.php";
_load_language_file("/website_code/php/import/fileupload.inc");
-if(in_array($_FILES['filenameuploaded']['type'],$xerte_toolkits_site->mimetypes)){
+if(in_array($_FILES['filenameuploaded']['type'],$xerte_toolkits_site->mimetypes) && apply_filters('editor_upload_file', $_FILES)){
if($_FILES['filenameuploaded']['type']=="text/html"){--- languages/en-GB/website_code/php/import/import.inc.orig 2015-08-20 17:25:04.736388237 +0100
+++ languages/en-GB/website_code/php/import/import.inc 2015-08-19 13:51:53.503220930 +0100
@@ -34,5 +34,7 @@
define("IMPORT_FAILED", "Import failed with a generic error. Is the file too large?");
+ define("IMPORT_CHECKS_FAILED", "Checks on the imported files have failed");
+
?>--- editor/elfinder/php/elFinder.class.php.orig 2015-07-29 23:17:23.000000000 +0100
+++ editor/elfinder/php/elFinder.class.php 2015-08-20 15:40:43.784550292 +0100
@@ -9,6 +9,9 @@
* @author Troex Nevelin
* @author Alexey Sukhotin
**/
+
+require_once("../../../plugins.php");
+
class elFinder {
/**
@@ -929,6 +932,12 @@
}
$tmpname = $files['tmp_name'][$i];
+
+ if (!apply_filters('editor_upload_file', array('name' => array($name), 'tmp_name' => array($tmpname)))) {
+ $result['warning'] = $this->error(self::ERROR_UPLOAD_FILE, $name, self::ERROR_UPLOAD_TRANSFER, 'Virus checks');
+ $this->uploadDebug = 'Upload error: file failed virus checks';
+ break;
+ }
if (($fp = fopen($tmpname, 'rb')) == false) {
$result['warning'] = $this->error(self::ERROR_UPLOAD_FILE, $name, self::ERROR_UPLOAD_TRANSFER);--- plugins/file_uploading-clamav.php.orig 2015-07-29 23:17:23.000000000 +0100
+++ plugins/file_uploading-clamav.php 2015-08-20 15:54:21.364894641 +0100
@@ -32,17 +32,42 @@
*/
function virus_check_file() {
$args = func_get_args();
- $files = $args[0]; /* $_FILES like */
+ $files = array();
- if(Xerte_Validate_VirusScanClamAv::canRun()){
- foreach($files as $file) {
- $validator = new Xerte_Validate_VirusScanClamAv();
- if(!$validator->isValid($file['tmp_name'])) {
- die("Possible virus found in upload; Consult server log files for more information.");
+ if(!Xerte_Validate_VirusScanClamAv::canRun() || !is_array($args[0])) {
+ return $args[0];
+ }
+
+ /*
+ * The file names may be supplied in two slightly different formats.
+ * As such we dig out the real and temporary file names, and store them
+ * in a local array for easier processing.
+ */
+
+ if (isset($args[0]['filenameuploaded'])) {
+ $files['file_name'][] = $args[0]['filenameuploaded']['name'];
+ $files['temp_name'][] = $args[0]['filenameuploaded']['tmp_name'];
+ }
+ else {
+ foreach ($args[0]['name'] as $key => $name) {
+ $files['file_name'][] = $name;
+ $files['temp_name'][] = $args[0]['tmp_name'][$key];
+ }
+ }
+
+ foreach($files['temp_name'] as $key => $file) {
+ $validator = new Xerte_Validate_VirusScanClamAv();
+ if(!$validator->isValid($file)) {
+ _debug("Invalid file {$files['file_name'][$key]} uploaded - virus found");
+ error_log("Invalid file {$files['file_name'][$key]} ($file) uploaded - virus found");
+ if (file_exists($file)) {
+ unlink($file);
}
+ return false;
}
}
- return $files;
+
+ return $args[0];
}
--- plugins/file_uploading-extension-check.php.orig 2015-07-29 23:17:23.000000000 +0100
+++ plugins/file_uploading-extension-check.php 2015-08-20 17:28:38.565418655 +0100
@@ -26,22 +26,43 @@
Xerte_Validate_FileExtension::$BLACKLIST = 'php,php5,pl,cgi,exe,vbs,pif,application,gadget,msi,msp,com,scr,hta,htaccess,ini,cpl,msc,jar,bat,cmd,vb,vbe,jsp,jse,ws,wsf,wsc,wsh,ps1,ps1xml,ps2,ps2xml,psc1,psc2,msh,msh1,msh2,mshxml,msh1xml,msh2xml,scf,lnk,inf,reg,docm,dotm,xlsm,xltm,xlam,pptm,potm,ppam,ppsm,sldm';
$args = func_get_args();
- $files = $args[0];
-
- if(!Xerte_Validate_FileExtension::canRun()) {
- return $files;
+ $files = array();
+
+ if(!Xerte_Validate_FileExtension::canRun() || !is_array($args[0])) {
+ return $args[0];
}
+ /*
+ * The file names may be supplied in two slightly different formats.
+ * As such we dig out the real and temporary file names, and store them
+ * in a local array for easier processing.
+ */
+
+ if (isset($args[0]['filenameuploaded'])) {
+ $files['file_name'][] = $args[0]['filenameuploaded']['name'];
+ $files['temp_name'][] = $args[0]['filenameuploaded']['tmp_name'];
+ }
+ else {
+ foreach ($args[0]['name'] as $key => $name) {
+ $files['file_name'][] = $name;
+ $files['temp_name'][] = $args[0]['tmp_name'][$key];
+ }
+ }
- foreach($files as $file) {
+ foreach($files['file_name'] as $key => $file) {
$validator = new Xerte_Validate_FileExtension();
- if(!$validator->isValid($file['name'])) {
- _debug("Invalid file {$file['name']} type uploaded - matches blacklist");
+ if(!$validator->isValid($file)) {
+ $real_path = $files['temp_name'][$key];
+ _debug("Invalid file {$file} type uploaded - matches blacklist");
+ error_log("Invalid file extension found for $file ($real_path)");
+ if (file_exists($real_path)) {
+ unlink($real_path);
+ }
return false;
}
}
- return $files;
+ return $args[0];
}
add_filter('editor_upload_file', 'filter_by_extension_name');--- plugins/file_uploading-mimetype.php.orig 2015-07-29 23:17:23.000000000 +0100
+++ plugins/file_uploading-mimetype.php 2015-08-20 15:56:54.227817332 +0100
@@ -25,24 +25,42 @@
function filter_by_mimetype() {
$args = func_get_args();
- $files = $args[0];
- _debug($args);
- foreach($files as $file) {
- _debug("Checking {$file['name']} for mimetype etc");
-
- $user_filename = $file['name'];
- $php_upload_filename = $file['tmp_name'];
-
- $validator = new Xerte_Validate_FileMimeType();
- if($validator->isValid($php_upload_filename)) {
- _debug("Mime check of $php_upload_filename ($user_filename) - ok");
+ $files = array();
+
+ if(!Xerte_Validate_FileMimeType::canRun() || !is_array($args[0])) {
+ return $args[0];
+ }
+
+ /*
+ * The file names may be supplied in two slightly different formats.
+ * As such we dig out the real and temporary file names, and store them
+ * in a local array for easier processing.
+ */
+
+ if (isset($args[0]['filenameuploaded'])) {
+ $files['file_name'][] = $args[0]['filenameuploaded']['name'];
+ $files['temp_name'][] = $args[0]['filenameuploaded']['tmp_name'];
+ }
+ else {
+ foreach ($args[0]['name'] as $key => $name) {
+ $files['file_name'][] = $name;
+ $files['temp_name'][] = $args[0]['tmp_name'][$key];
}
- else {
- _debug("Mime check of $php_upload_filename ($user_filename) failed. ");
+ }
+
+ foreach($files['temp_name'] as $key => $file) {
+ $validator = new Xerte_Validate_FileMimeType();
+ if(!$validator->isValid($file)) {
+ _debug("Mime check of {$files['file_name'][$key]} failed.");
+ error_log("Mime check of {$files['file_name'][$key]} ($file) failed");
+ if (file_exists($file)) {
+ unlink($file);
+ }
return false;
}
}
- return $files;
+
+ return $args[0];
}
if(Xerte_Validate_FileMimeType::canRun()) {--- website_code/php/import/import.php.orig 2015-07-29 23:17:24.000000000 +0100
+++ website_code/php/import/import.php 2015-08-20 15:34:57.578814441 +0100
@@ -28,6 +28,7 @@
*/
require_once("../../../config.php");
+require_once("../../../plugins.php");
_load_language_file("/website_code/php/import/import.inc");
@@ -39,6 +40,7 @@
$likelihood_array = array();
$delete_folder_array = array();
$delete_file_array = array();
+$check_file_array = array();
$rlt_name = "";
/**
@@ -527,12 +529,28 @@
}
+ $check_file_array['name'][] = $file_to_create[0];
+ $check_file_array['tmp_name'][] = $xerte_toolkits_site->import_path . $this_dir . $file_to_create[0];
}
$zip->close();
unlink($new_file_name);
+ /* Check the imported files. */
+ if (!apply_filters('editor_upload_file', $check_file_array)) {
+ delete_loop($xerte_toolkits_site->import_path . $this_dir);
+ while($delete_file = array_pop($delete_file_array)){
+ unlink($delete_file);
+ }
+
+ while($delete_folder = array_pop($delete_folder_array)){
+ rmdir($delete_folder);
+ }
+ rmdir($xerte_toolkits_site->import_path . $this_dir);
+ die(IMPORT_CHECKS_FAILED . ".****");
+ }
+
/*
* use the template attributes to make the folders required and name them accordingly
*John. |
|
Please Log in or Create an account to join the conversation. |
Moderators: ronm, julten, JohnSmith
Time to create page: 0.041 seconds