Fixed security issue: the attachments were visible by anybody (by forming URLs manually), whatever the allowed organizations. The change requires the execution of the setup/migration procedure.

git-svn-id: http://svn.code.sf.net/p/itop/code/trunk@1591 a333f486-631f-4898-b8df-5754b55c2be0

modules/itop-attachments/ajax.attachment.php

@@ -63,25 +63,25 @@ try
 		{
 			try
 			{
-		   		$oDoc = utils::ReadPostedDocument('file');
-		   		$oAttachment = MetaModel::NewObject('Attachment');
-		   		$oAttachment->Set('expire', time() + 3600); // one hour...
-		   		$oAttachment->Set('temp_id', $sTempId);
-		   		$oAttachment->Set('item_class', $sObjClass);
-		   		$oAttachment->Set('item_id', 0);
-		   		$oAttachment->Set('contents', $oDoc);
-		   		$iAttId = $oAttachment->DBInsert();
-		   		
-		   		$aResult['msg'] = $oDoc->GetFileName();
-		   		$aResult['icon'] = utils::GetAbsoluteUrlAppRoot().AttachmentPlugIn::GetFileIcon($oDoc->GetFileName());
-		   		$aResult['att_id'] = $iAttId;
+				$oDoc = utils::ReadPostedDocument('file');
+				$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['msg'] = $oDoc->GetFileName();
+				$aResult['icon'] = utils::GetAbsoluteUrlAppRoot().AttachmentPlugIn::GetFileIcon($oDoc->GetFileName());
+				$aResult['att_id'] = $iAttId;
 			}
 			catch (FileUploadException $e)
 			{
 					$aResult['error'] = $e->GetMessage();
 			}
-	   }
-	   $oPage->add(json_encode($aResult));
+		}
+		$oPage->add(json_encode($aResult));
 		break;
 	
 	case 'remove':

modules/itop-attachments/model.attachments.php

@@ -34,7 +34,7 @@ class Attachment extends DBObject
 	{
 		$aParams = array
 		(
-			"category" => "addon",
+			"category" => "addon,bizmodel",
 			"key_type" => "autoincrement",
 			"name_attcode" => array('item_class', 'temp_id'),
 			"state_attcode" => "",
@@ -51,15 +51,107 @@ class Attachment extends DBObject
 
 		MetaModel::Init_AddAttribute(new AttributeString("item_class", array("allowed_values"=>null, "sql"=>"item_class", "default_value"=>"", "is_null_allowed"=>false, "depends_on"=>array())));
 		MetaModel::Init_AddAttribute(new AttributeString("item_id", array("allowed_values"=>null, "sql"=>"item_id", "default_value"=>"", "is_null_allowed"=>true, "depends_on"=>array())));
+		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())));
 
 		MetaModel::Init_AddAttribute(new AttributeBlob("contents", array("depends_on"=>array())));
 
-		MetaModel::Init_SetZListItems('details', array('temp_id', 'item_class', 'item_id'));
+		MetaModel::Init_SetZListItems('details', array('temp_id', 'item_class', 'item_id', 'item_org_id'));
 		MetaModel::Init_SetZListItems('advanced_search', array('temp_id', 'item_class', 'item_id'));
 		MetaModel::Init_SetZListItems('standard_search', array('temp_id', 'item_class', 'item_id'));
 		MetaModel::Init_SetZListItems('list', array('temp_id', 'item_class', 'item_id'));
 	}
 
+	/**
+	 * Maps the given context parameter name to the appropriate filter/search code for this class
+	 * @param string $sContextParam Name of the context parameter, e.g. 'org_id'
+	 * @return string Filter code, e.g. 'customer_id'
+	 */
+	public static function MapContextParam($sContextParam)
+	{
+		if ($sContextParam == 'org_id')
+		{
+			return 'item_org_id';
+		}
+		else
+		{
+			return null;
+		}
+	}
+
+	/**
+	 * Set/Update all of the '_item' fields
+	 * @param object $oItem Container item
+	 * @return void
+	 */
+	public function SetItem($oItem, $bUpdateOnChange = false)
+	{
+		$sClass = get_class($oItem);
+		$iItemId = $oItem->GetKey();
+
+ 		$this->Set('item_class', $sClass);
+ 		$this->Set('item_id', $iItemId);
+
+		$aCallSpec = array($sClass, 'MapContextParam');
+		if (is_callable($aCallSpec))
+		{
+			$sAttCode = call_user_func($aCallSpec, 'org_id'); // Returns null when there is no mapping for this parameter					
+			if (MetaModel::IsValidAttCode($sClass, $sAttCode))
+			{
+				$iOrgId = $oItem->Get($sAttCode);
+				if ($iOrgId > 0)
+				{
+					if ($iOrgId != $this->Get('item_org_id'))
+					{
+						$this->Set('item_org_id', $iOrgId);
+						if ($bUpdateOnChange)
+						{
+							$this->DBUpdate();
+						}
+					}
+				}
+			}
+		}
+	}
+
+	/**
+	 * Give a default value for item_org_id (if relevant...)
+	 * @return void
+	 */
+	public function SetDefaultOrgId()
+	{
+		// First check that the organization CAN be fetched from the target class
+		//
+		$sClass = $this->Get('item_class');
+		$aCallSpec = array($sClass, 'MapContextParam');
+		if (is_callable($aCallSpec))
+		{
+			$sAttCode = call_user_func($aCallSpec, 'org_id'); // Returns null when there is no mapping for this parameter					
+			if (MetaModel::IsValidAttCode($sClass, $sAttCode))
+			{
+				// Second: check that the organization CAN be fetched from the current user
+				//
+				if (MetaModel::IsValidClass('Person'))
+				{
+					$aCallSpec = array($sClass, 'MapContextParam');
+					if (is_callable($aCallSpec))
+					{
+						$sAttCode = call_user_func($aCallSpec, 'org_id'); // Returns null when there is no mapping for this parameter					
+						if (MetaModel::IsValidAttCode($sClass, $sAttCode))
+						{
+							// OK - try it
+							//
+							$oCurrentPerson = MetaModel::GetObject('Person', UserRights::GetContactId(), false);
+							if ($oCurrentPerson)
+							{
+						 		$this->Set('item_org_id', $oCurrentPerson->Get($sAttCode));
+						 	}
+						}
+					}
+				}
+			}
+		}
+	}
+
 	// Todo - implement a cleanup function (see a way to do that generic !)
 }
 
@@ -181,6 +273,16 @@ class AttachmentPlugIn implements iApplicationUIExtension, iApplicationObjectExt
 
 	public function OnDBUpdate($oObject, $oChange = null)
 	{
+		if ($this->IsTargetObject($oObject))
+		{
+			// Get all current attachments
+			$oSearch = DBObjectSearch::FromOQL("SELECT Attachment WHERE item_class = :class AND item_id = :item_id");
+			$oSet = new DBObjectSet($oSearch, array(), array('class' => get_class($oObject), 'item_id' => $oObject->GetKey()));
+			while ($oAttachment = $oSet->Fetch())
+			{
+				$oAttachment->SetItem($oObject, true /*updateonchange*/);
+			}			
+		}
 	}
 	
 	public function OnDBInsert($oObject, $oChange = null)
@@ -424,7 +526,7 @@ EOF
 					}
 					else
 					{
-						$oAttachment->Set('item_id', $oObject->GetKey());
+						$oAttachment->SetItem($oObject);
 						$oAttachment->Set('temp_id', '');
 						$oAttachment->DBUpdate();
 						// temporary attachment confirmed, list it in the history

modules/itop-attachments/module.attachments.php

@@ -30,6 +30,7 @@ SetupWebPage::AddModule(
 		),
 		'mandatory' => false,
 		'visible' => true,
+		'installer' => 'AttachmentInstaller',
 
 		// Components
 		//
@@ -63,4 +64,61 @@ SetupWebPage::AddModule(
 		),
 	)
 );
-?>
+
+// Module installation handler
+//
+class AttachmentInstaller extends ModuleInstallerAPI
+{
+	public static function BeforeWritingConfig(Config $oConfiguration)
+	{
+		// If you want to override/force some configuration values, do it here
+		return $oConfiguration;
+	}
+
+	/**
+	 * Handler called before creating or upgrading the database schema
+	 * @param $oConfiguration Config The new configuration of the application
+	 * @param $sPreviousVersion string PRevious version number of the module (empty string in case of first install)
+	 * @param $sCurrentVersion string Current version number of the module
+	 */
+	public static function BeforeDatabaseCreation(Config $oConfiguration, $sPreviousVersion, $sCurrentVersion)
+	{
+		// If you want to migrate data from one format to another, do it here
+	}
+	
+	/**
+	 * Handler called after the creation/update of the database schema
+	 * @param $oConfiguration Config The new configuration of the application
+	 * @param $sPreviousVersion string PRevious version number of the module (empty string in case of first install)
+	 * @param $sCurrentVersion string Current version number of the module
+	 */
+	public static function AfterDatabaseCreation(Config $oConfiguration, $sPreviousVersion, $sCurrentVersion)
+	{
+		// For each record having item_org_id unset,
+		//    get the org_id from the container object 
+		//
+		// Prerequisite: change null into 0 (workaround to the fact that we cannot use IS NULL in OQL)
+		SetupWebPage::log_info("Initializing attachment/item_org_id - null to zero"); 
+		$sRepair = "UPDATE `Attachment` SET `item_org_id` = 0 WHERE `item_org_id` IS NULL";
+		CMDBSource::Query($sRepair);
+
+		SetupWebPage::log_info("Initializing attachment/item_org_id - zero to the container"); 
+		$oSearch = DBObjectSearch::FromOQL('SELECT Attachment WHERE item_org_id = 0');
+		$oSet = new DBObjectSet($oSearch);
+		$iUpdated = 0;
+		while ($oAttachment = $oSet->Fetch())
+		{
+			$oContainer = MetaModel::GetObject($oAttachment->Get('item_class'), $oAttachment->Get('item_id'), false /* must be found */, true /* allow all data */);
+			if ($oContainer)
+			{
+				$oAttachment->SetItem($oContainer, true /*updateonchange*/);
+				$iUpdated++;
+			}
+		}
+
+		SetupWebPage::log_info("Initializing attachment/item_org_id - $iUpdated records have been adjusted"); 
+	}
+}
+
+
+?>

pages/ajax.render.php

@@ -592,16 +592,17 @@ function DownloadDocument(WebPage $oPage, $sClass, $id, $sAttCode, $sContentDisp
 {
 	try
 	{
-		$oObj = MetaModel::GetObject($sClass, $id);
-		if (is_object($oObj))
+		$oObj = MetaModel::GetObject($sClass, $id, false, false);
+		if (!is_object($oObj))
 		{
-			$oDocument = $oObj->Get($sAttCode);
-			if (is_object($oDocument))
-			{
-				$oPage->SetContentType($oDocument->GetMimeType());
-				$oPage->SetContentDisposition($sContentDisposition,$oDocument->GetFileName());
-				$oPage->add($oDocument->GetData());
-			}
+			throw new Exception("Invalid id ($id) for class '$sClass' - the object does not exist or you are not allowed to view it");
+		}
+		$oDocument = $oObj->Get($sAttCode);
+		if (is_object($oDocument))
+		{
+			$oPage->SetContentType($oDocument->GetMimeType());
+			$oPage->SetContentDisposition($sContentDisposition,$oDocument->GetFileName());
+			$oPage->add($oDocument->GetData());
 		}
 	}
 	catch(Exception $e)