From 32ce26aa7dcd107c030fdcdaef2f2e85804e58e5 Mon Sep 17 00:00:00 2001 From: Denis Flaven Date: Tue, 5 Apr 2016 16:15:29 +0000 Subject: [PATCH] Fix for potential XSS vulnerability on uploaded file names. To be further tested before retrofitting in branches. SVN:trunk[3985] --- application/cmdbabstract.class.inc.php | 2 +- core/ormdocument.class.inc.php | 8 +- .../2.x/itop-attachments/ajax.attachment.php | 114 +----------------- .../2.x/itop-attachments/main.attachments.php | 6 +- pages/ajax.render.php | 2 +- 5 files changed, 10 insertions(+), 122 deletions(-) diff --git a/application/cmdbabstract.class.inc.php b/application/cmdbabstract.class.inc.php index 2b4dc196e..e242905ce 100644 --- a/application/cmdbabstract.class.inc.php +++ b/application/cmdbabstract.class.inc.php @@ -1868,7 +1868,7 @@ EOF $iMaxFileSize = utils::ConvertToBytes(ini_get('upload_max_filesize')); $sHTMLValue = "\n"; $sHTMLValue .= "\n"; - $sHTMLValue .= "$sFileName
\n"; + $sHTMLValue .= "".htmlentities($sFileName, ENT_QUOTES, 'UTF-8')."
\n"; $sHTMLValue .= " {$sValidationSpan}{$sReloadSpan}\n"; break; diff --git a/core/ormdocument.class.inc.php b/core/ormdocument.class.inc.php index 72a897f05..d447fc0fe 100644 --- a/core/ormdocument.class.inc.php +++ b/core/ormdocument.class.inc.php @@ -90,12 +90,12 @@ class ormDocument { // If the filename is not empty, display it, this is used // by the creation wizard while the file has not yet been uploaded - $sResult = $this->GetFileName(); + $sResult = htmlentities($this->GetFileName(), ENT_QUOTES, 'UTF-8'); } else { $data = $this->GetData(); - $sResult = $this->GetFileName().' [ '.$this->GetMimeType().', size: '.strlen($data).' byte(s) ]
'; + $sResult = htmlentities($this->GetFileName(), ENT_QUOTES, 'UTF-8').' [ '.$this->GetMimeType().', size: '.strlen($data).' byte(s) ]
'; } return $sResult; } @@ -106,7 +106,7 @@ class ormDocument */ public function GetDisplayLink($sClass, $Id, $sAttCode) { - return "".$this->GetFileName()."\n"; + return "".htmlentities($this->GetFileName(), ENT_QUOTES, 'UTF-8')."\n"; } /** @@ -115,7 +115,7 @@ class ormDocument */ public function GetDownloadLink($sClass, $Id, $sAttCode) { - return "".$this->GetFileName()."\n"; + return "".htmlentities($this->GetFileName(), ENT_QUOTES, 'UTF-8')."\n"; } /** diff --git a/datamodels/2.x/itop-attachments/ajax.attachment.php b/datamodels/2.x/itop-attachments/ajax.attachment.php index 418edb790..754cb6081 100755 --- a/datamodels/2.x/itop-attachments/ajax.attachment.php +++ b/datamodels/2.x/itop-attachments/ajax.attachment.php @@ -74,7 +74,7 @@ try $oAttachment->Set('contents', $oDoc); $iAttId = $oAttachment->DBInsert(); - $aResult['msg'] = $oDoc->GetFileName(); + $aResult['msg'] = htmlentities($oDoc->GetFileName(), ENT_QUOTES, 'UTF-8'); $aResult['icon'] = utils::GetAbsoluteUrlAppRoot().AttachmentPlugIn::GetFileIcon($oDoc->GetFileName()); $aResult['att_id'] = $iAttId; $aResult['preview'] = $oDoc->IsPreviewAvailable() ? 'true' : 'false'; @@ -96,118 +96,6 @@ try $oAttachment->DBDelete(); } break; - - - case 'cke_img_upload': - // Image uploaded via CKEditor - $aResult = array( - 'uploaded' => 0, - 'fileName' => '', - 'url' => '', - 'icon' => '', - 'msg' => '', - 'att_id' => 0, - 'preview' => 'false', - ); - - $sObjClass = stripslashes(utils::ReadParam('obj_class', '', false, 'class')); - $sTempId = utils::ReadParam('temp_id', ''); - if (empty($sObjClass)) - { - $aResult['error'] = "Missing argument 'obj_class'"; - } - elseif (empty($sTempId)) - { - $aResult['error'] = "Missing argument 'temp_id'"; - } - else - { - try - { - $oDoc = utils::ReadPostedDocument('upload'); - $oAttachment = MetaModel::NewObject('Attachment'); - $oAttachment->Set('expire', time() + 3600); // one hour... - $oAttachment->Set('temp_id', $sTempId); - $oAttachment->Set('item_class', $sObjClass); - $oAttachment->SetDefaultOrgId(); - $oAttachment->Set('contents', $oDoc); - $iAttId = $oAttachment->DBInsert(); - - $aResult['uploaded'] = 1; - $aResult['msg'] = $oDoc->GetFileName(); - $aResult['fileName'] = $oDoc->GetFileName(); - $aResult['url'] = utils::GetAbsoluteUrlAppRoot().ATTACHMENT_DOWNLOAD_URL.$iAttId; - $aResult['icon'] = utils::GetAbsoluteUrlAppRoot().AttachmentPlugIn::GetFileIcon($oDoc->GetFileName()); - $aResult['att_id'] = $iAttId; - $aResult['preview'] = $oDoc->IsPreviewAvailable() ? 'true' : 'false'; - } - catch (FileUploadException $e) - { - $aResult['error'] = $e->GetMessage(); - } - } - $oPage->add(json_encode($aResult)); - break; - - case 'cke_browse': - $oPage = new NiceWebPage('Browse for image...'); - $oPage->add_linked_stylesheet(utils::GetAbsoluteUrlModulesRoot().'itop-attachments/css/magnific-popup.css'); - $oPage->add_linked_script(utils::GetAbsoluteUrlModulesRoot().'itop-attachments/js/jquery.magnific-popup.min.js'); - $sImgUrl = utils::GetAbsoluteUrlAppRoot().ATTACHMENT_DOWNLOAD_URL; - $oPage->add_script( -<< 1 ) ? match[1] : null; - } - // Simulate user action of selecting a file to be returned to CKEditor. - function returnFileUrl(iAttId, sAltText) { - - var funcNum = getUrlParam( 'CKEditorFuncNum' ); - var fileUrl = '$sImgUrl'+iAttId; - window.opener.CKEDITOR.tools.callFunction( funcNum, fileUrl, function() { - // Get the reference to a dialog window. - var dialog = this.getDialog(); - // Check if this is the Image Properties dialog window. - if ( dialog.getName() == 'image' ) { - // Get the reference to a text field that stores the "alt" attribute. - var element = dialog.getContentElement( 'info', 'txtAlt' ); - // Assign the new value. - if ( element ) - element.setValue(sAltText); - } - // Return "false" to stop further execution. In such case CKEditor will ignore the second argument ("fileUrl") - // and the "onSelect" function assigned to the button that called the file manager (if defined). - // return false; - } ); - window.close(); - } -EOF - ); - $oPage->add_ready_script( -<< $sTempId, 'obj_class' => $sClass, 'obj_id' => $iObjectId)); - while($oAttachment = $oSet->Fetch()) - { - $oDoc = $oAttachment->Get('contents'); - if ($oDoc->GetMainMimeType() == 'image') - { - $sDocName = addslashes(htmlentities($oDoc->GetFileName(), ENT_QUOTES, 'UTF-8')); - $iAttId = $oAttachment->GetKey(); - $oPage->add("
\"$sDocName\"
"); - } - } - break; default: $oPage->p("Missing argument 'operation'"); diff --git a/datamodels/2.x/itop-attachments/main.attachments.php b/datamodels/2.x/itop-attachments/main.attachments.php index 0483c3428..2f47d503c 100755 --- a/datamodels/2.x/itop-attachments/main.attachments.php +++ b/datamodels/2.x/itop-attachments/main.attachments.php @@ -275,7 +275,7 @@ EOF { $iAttId = $oAttachment->GetKey(); $oDoc = $oAttachment->Get('contents'); - $sFileName = $oDoc->GetFileName(); + $sFileName = htmlentities($oDoc->GetFileName(), ENT_QUOTES, 'UTF-8'); $sIcon = utils::GetAbsoluteUrlAppRoot().AttachmentPlugIn::GetFileIcon($sFileName); $sPreview = $oDoc->IsPreviewAvailable() ? 'true' : 'false'; $sDownloadLink = utils::GetAbsoluteUrlAppRoot().ATTACHMENT_DOWNLOAD_URL.$iAttId; @@ -303,7 +303,7 @@ EOF // Display them $iAttId = $oAttachment->GetKey(); $oDoc = $oAttachment->Get('contents'); - $sFileName = $oDoc->GetFileName(); + $sFileName = htmlentities($oDoc->GetFileName(), ENT_QUOTES, 'UTF-8'); $sIcon = utils::GetAbsoluteUrlAppRoot().AttachmentPlugIn::GetFileIcon($sFileName); $sDownloadLink = utils::GetAbsoluteUrlAppRoot().ATTACHMENT_DOWNLOAD_URL.$iAttId; $sPreview = $oDoc->IsPreviewAvailable() ? 'true' : 'false'; @@ -439,7 +439,7 @@ EOF { $iAttId = $oAttachment->GetKey(); $oDoc = $oAttachment->Get('contents'); - $sFileName = $oDoc->GetFileName(); + $sFileName = htmlentities($oDoc->GetFileName(), ENT_QUOTES, 'UTF-8'); $sIcon = utils::GetAbsoluteUrlAppRoot().AttachmentPlugIn::GetFileIcon($sFileName); $sPreview = $oDoc->IsPreviewAvailable() ? 'true' : 'false'; $sDownloadLink = utils::GetAbsoluteUrlAppRoot().ATTACHMENT_DOWNLOAD_URL.$iAttId; diff --git a/pages/ajax.render.php b/pages/ajax.render.php index e07067624..9c806f2a6 100644 --- a/pages/ajax.render.php +++ b/pages/ajax.render.php @@ -2274,7 +2274,7 @@ EOF $iAttId = $oAttachment->DBInsert(); $aResult['uploaded'] = 1; - $aResult['msg'] = $oDoc->GetFileName(); + $aResult['msg'] = htmlentities($oDoc->GetFileName(), ENT_QUOTES, 'UTF-8'); $aResult['fileName'] = $oDoc->GetFileName(); $aResult['url'] = utils::GetAbsoluteUrlAppRoot().INLINEIMAGE_DOWNLOAD_URL.$iAttId.'&s='.$oAttachment->Get('secret'); if (is_array($aDimensions))