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.

SVN:trunk[1591]
This commit is contained in:
Romain Quetiez
2011-09-22 09:04:12 +00:00
parent 0de75db474
commit da2b8ab4c0
4 changed files with 188 additions and 27 deletions

View File

@@ -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':

View File

@@ -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

View File

@@ -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");
}
}
?>

View File

@@ -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)