Przeglądaj źródła

Prevent access to *any* InlineImage by just guessing its identifier, now an additional "secret" is needed, making it much harder to guess (but not 100% impossible, beware !)

git-svn-id: http://svn.code.sf.net/p/itop/code/trunk@3927 a333f486-631f-4898-b8df-5754b55c2be0
dflaven 9 lat temu
rodzic
commit
378fb38f3f

+ 2 - 1
core/htmlsanitizer.class.inc.php

@@ -303,10 +303,11 @@ class HTMLDOMSanitizer extends HTMLSanitizer
 	{
 		$sSrc = $oElement->getAttribute('src');
 		$sDownloadUrl = str_replace(array('.', '?'), array('\.', '\?'), INLINEIMAGE_DOWNLOAD_URL); // Escape . and ?
-		$sUrlPattern = '|'.$sDownloadUrl.'([0-9]+)|';
+		$sUrlPattern = '|'.$sDownloadUrl.'([0-9]+)&s=([0-9a-f]+)|';
 		if (preg_match($sUrlPattern, $sSrc, $aMatches))
 		{
 			$oElement->setAttribute('data-img-id', $aMatches[1]);
+			$oElement->setAttribute('data-img-secret', $aMatches[2]);
 		}
 	}
 	

+ 8 - 3
core/inlineimage.class.inc.php

@@ -16,7 +16,7 @@
 //   You should have received a copy of the GNU Affero General Public License
 //   along with iTop. If not, see <http://www.gnu.org/licenses/>
 
-define('INLINEIMAGE_DOWNLOAD_URL', 'pages/ajax.render.php?operation=download_document&class=InlineImage&field=contents&id=');
+define('INLINEIMAGE_DOWNLOAD_URL', 'pages/ajax.render.php?operation=download_inlineimage&id=');
 
 /**
  * Persistent classes (internal): store images referenced inside HTML formatted text fields
@@ -53,7 +53,7 @@ class InlineImage extends DBObject
 		MetaModel::Init_AddAttribute(new AttributeObjectKey("item_id", array("class_attcode"=>'item_class', "allowed_values"=>null, "sql"=>'item_id', "is_null_allowed"=>true, "depends_on"=>array(), "always_load_in_tables"=>false)));
 		MetaModel::Init_AddAttribute(new AttributeInteger("item_org_id", array("allowed_values"=>null, "sql"=>'item_org_id', "default_value"=>'0', "is_null_allowed"=>true, "depends_on"=>array(), "always_load_in_tables"=>false)));
 		MetaModel::Init_AddAttribute(new AttributeBlob("contents", array("is_null_allowed"=>false, "depends_on"=>array(), "always_load_in_tables"=>false)));
-
+		MetaModel::Init_AddAttribute(new AttributeString("secret", array("allowed_values"=>null, "sql"=>'secret', "default_value"=>'', "is_null_allowed"=>false, "depends_on"=>array(), "always_load_in_tables"=>false)));
 
 
 		MetaModel::Init_SetZListItems('details', array('temp_id', 'item_class', 'item_id', 'item_org_id'));
@@ -214,9 +214,14 @@ class InlineImage extends DBObject
 			foreach($aMatches as $aImgInfo)
 			{
 				$sImgTag = $aImgInfo[0][0];
+				$sSecret = '';
+				if (preg_match('/data-img-secret="([0-9a-f]+)"/', $sImgTag, $aSecretMatches))
+				{
+					$sSecret = '&s='.$aSecretMatches[1];
+				}
 				$sAttId = $aImgInfo[2][0];
 	
-				$sNewImgTag = preg_replace('/src="[^"]+"/', 'src="'.$sUrl.$sAttId.'"', $sImgTag); // preserve other attributes
+				$sNewImgTag = preg_replace('/src="[^"]+"/', 'src="'.$sUrl.$sAttId.$sSecret.'"', $sImgTag); // preserve other attributes
 				$aNeedles[] = $sImgTag;
 				$aReplacements[] = $sNewImgTag;
 			}

+ 32 - 11
pages/ajax.render.php

@@ -771,7 +771,7 @@ try
 		case 'display_document':
 		$id = utils::ReadParam('id', '');
 		$sField = utils::ReadParam('field', '');
-		if (!empty($sClass) && !empty($id) && !empty($sField))
+		if (!empty($sClass) && ($sClass != 'InlineImage') && !empty($id) && !empty($sField))
 		{
 			DownloadDocument($oPage, $sClass, $id, $sField, 'inline');
 		}
@@ -781,7 +781,7 @@ try
 		$id = utils::ReadParam('id', '');
 		$sField = utils::ReadParam('field', '');
 		$iCacheSec = (int) utils::ReadParam('cache', 0);
-		if (!empty($sClass) && !empty($id) && !empty($sField))
+		if (!empty($sClass) && ($sClass != 'InlineImage') && !empty($id) && !empty($sField))
 		{
 			DownloadDocument($oPage, $sClass, $id, $sField, 'attachment');
 			if ($iCacheSec > 0)
@@ -2253,15 +2253,13 @@ EOF
 						$oAttachment->Set('item_class', $sObjClass);
 						$oAttachment->SetDefaultOrgId();
 						$oAttachment->Set('contents', $oDoc);
+						$oAttachment->Set('secret', sprintf ('%06x', mt_rand(0, 0xFFFFFF))); // something not easy to guess
 						$iAttId = $oAttachment->DBInsert();
 			
 						$aResult['uploaded'] = 1;
 						$aResult['msg'] = $oDoc->GetFileName();
 						$aResult['fileName'] = $oDoc->GetFileName();
-						$aResult['url'] = utils::GetAbsoluteUrlAppRoot().INLINEIMAGE_DOWNLOAD_URL.$iAttId;
-						$aResult['icon'] = utils::GetAbsoluteUrlAppRoot().AttachmentPlugIn::GetFileIcon($oDoc->GetFileName());
-						$aResult['att_id'] = $iAttId;
-						$aResult['preview'] = $oDoc->IsPreviewAvailable() ? 'true' : 'false';
+						$aResult['url'] = utils::GetAbsoluteUrlAppRoot().INLINEIMAGE_DOWNLOAD_URL.$iAttId.'&s='.$oAttachment->Get('secret');
 						if (is_array($aDimensions))
 						{
 							$aResult['width'] = $aDimensions['width'];
@@ -2297,6 +2295,7 @@ EOF
 					$oAttachment->Set('item_class', $sObjClass);
 					$oAttachment->SetDefaultOrgId();
 					$oAttachment->Set('contents', $oDoc);
+					$oAttachment->Set('secret', sprintf ('%06x', mt_rand(0, 0xFFFFFF))); // something not easy to guess
 					$iAttId = $oAttachment->DBInsert();
 						
 				}
@@ -2360,10 +2359,10 @@ EOF
             return ( match && match.length > 1 ) ? match[1] : null;
         }
         // Simulate user action of selecting a file to be returned to CKEditor.
-        function returnFileUrl(iAttId, sAltText) {
+        function returnFileUrl(iAttId, sAltText, sSecret) {
 
             var funcNum = getUrlParam( 'CKEditorFuncNum' );
-            var fileUrl = '$sImgUrl'+iAttId;
+            var fileUrl = '$sImgUrl'+iAttId+'&s='+sSecret;
             window.opener.CKEDITOR.tools.callFunction( funcNum, fileUrl, function() {
                 // Get the reference to a dialog window.
                 var dialog = this.getDialog();
@@ -2410,13 +2409,28 @@ EOF
 					{
 						$sDocName = addslashes(htmlentities($oDoc->GetFileName(), ENT_QUOTES, 'UTF-8'));
 						$iAttId = $oAttachment->GetKey();
-						$oPage->add("<div style=\"float:left;margin:1em;text-align:center;\"><img class=\"img-picker\" style=\"max-width:300px;cursor:zoom-in\" href=\"{$sImgUrl}{$iAttId}\" alt=\"$sDocName\" title=\"$sDocName\" src=\"{$sImgUrl}{$iAttId}\"><br/><button onclick=\"returnFileUrl($iAttId, '$sDocName')\">$sInsertBtnLabel</button></div>");
+						$sSecret = $oAttachment->Get('secret');
+						$oPage->add("<div style=\"float:left;margin:1em;text-align:center;\"><img class=\"img-picker\" style=\"max-width:300px;cursor:zoom-in\" href=\"{$sImgUrl}{$iAttId}&s={$sSecret}\" alt=\"$sDocName\" title=\"$sDocName\" src=\"{$sImgUrl}{$iAttId}&s={$sSecret}\"><br/><button onclick=\"returnFileUrl($iAttId, '$sDocName', '$sSecret')\">$sInsertBtnLabel</button></div>");
 					}
 				}
 			}
 			$oPage->add("</fieldset></div>");
 			break;		
-		
+	
+		case 'download_inlineimage':
+			$id = utils::ReadParam('id', '');
+			$sSecret = utils::ReadParam('s', '');
+			$iCacheSec = (int) utils::ReadParam('cache', 0);
+			if (!empty($id) && !empty($sSecret))
+			{
+				DownloadDocument($oPage, 'InlineImage', $id, 'contents', 'attachment', 'secret', $sSecret);
+				if ($iCacheSec > 0)
+				{
+					$oPage->add_header("Expires: "); // Reset the value set in ajax_page
+					$oPage->add_header("Cache-Control: no-transform,public,max-age=$iCacheSec,s-maxage=$iCacheSec");
+				}
+			}
+			break;
 		default:
 		$oPage->p("Invalid query.");
 	}
@@ -2440,9 +2454,11 @@ catch (Exception $e)
  * @param mixed $id Identifier of the object
  * @param string $sAttCode Name of the attribute containing the document to download
  * @param string $sContentDisposition Either 'inline' or 'attachment'
+ * @param string $sSecretField The attcode of the field containing a "secret" to be provided in order to retrieve the file
+ * @param string $sSecretValue The value of the secret to be compared with the value of the attribute $sSecretField
  * @return none
  */   
-function DownloadDocument(WebPage $oPage, $sClass, $id, $sAttCode, $sContentDisposition = 'attachment')
+function DownloadDocument(WebPage $oPage, $sClass, $id, $sAttCode, $sContentDisposition = 'attachment', $sSecretField = null, $sSecretValue = null)
 {
 	try
 	{
@@ -2451,6 +2467,11 @@ function DownloadDocument(WebPage $oPage, $sClass, $id, $sAttCode, $sContentDisp
 		{
 			throw new Exception("Invalid id ($id) for class '$sClass' - the object does not exist or you are not allowed to view it");
 		}
+		if (($sSecretField != null) && ($oObj->Get($sSecretField) != $sSecretValue))
+		{
+			usleep(200);
+			throw new Exception("Invalid secret for class '$sClass' - the object does not exist or you are not allowed to view it");
+		}
 		$oDocument = $oObj->Get($sAttCode);
 		if (is_object($oDocument))
 		{