From eb403791528d026f6aca0097d337ec1e628eb237 Mon Sep 17 00:00:00 2001 From: Duncan Sommerville Date: Tue, 17 Feb 2015 11:44:31 -0500 Subject: [PATCH 1/3] SAAS-595 - Changed Zend validation and added sanitization in file import process to throw out bad track number metadata --- airtime_mvc/application/forms/EditAudioMD.php | 2 +- .../rest/controllers/MediaController.php | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/airtime_mvc/application/forms/EditAudioMD.php b/airtime_mvc/application/forms/EditAudioMD.php index 69ddbccde..9fc41e314 100644 --- a/airtime_mvc/application/forms/EditAudioMD.php +++ b/airtime_mvc/application/forms/EditAudioMD.php @@ -59,7 +59,7 @@ class Application_Form_EditAudioMD extends Zend_Form $track_number->class = 'input_text'; $track_number->setLabel('Track Number:') ->setFilters(array('StringTrim')) - ->setValidators(array(new Zend_Validate_Digits())); + ->setValidators(array(new Zend_Validate_Int())); $this->addElement($track_number); // Add genre field diff --git a/airtime_mvc/application/modules/rest/controllers/MediaController.php b/airtime_mvc/application/modules/rest/controllers/MediaController.php index 7a7810711..1b9712bd2 100644 --- a/airtime_mvc/application/modules/rest/controllers/MediaController.php +++ b/airtime_mvc/application/modules/rest/controllers/MediaController.php @@ -113,6 +113,8 @@ class Rest_MediaController extends Zend_Rest_Controller $file->save(); return; } else { + // Sanitize any incorrect metadata that slipped past validation + $this->sanitizeData($file, $whiteList); /* If full_path is set, the post request came from ftp. * Users are allowed to upload folders via ftp. If this is the case * we need to include the folder name with the file name, otherwise @@ -165,6 +167,9 @@ class Rest_MediaController extends Zend_Rest_Controller $file->save(); return; } else if ($file) { + // Sanitize any incorrect metadata that slipped past validation + $this->sanitizeData($file, $whiteList); + $file->fromArray($whiteList, BasePeer::TYPE_FIELDNAME); //Our RESTful API takes "full_path" as a field, which we then split and translate to match @@ -297,6 +302,18 @@ class Rest_MediaController extends Zend_Rest_Controller return true; } + /** + * We want to throw out invalid data and process the upload successfully + * at all costs, so check the whitelisted data and sanitize it if necessary + * @param CcFiles $file CcFiles object being uploaded + * @param array $whitelist array of whitelisted (modifiable) file fields + */ + private function sanitizeData($file, &$whitelist) { + if (!ctype_digit(strval($whitelist["track_number"]))) { + $file->setDbTrackNumber(null); + } + } + private function processUploadedFile($callbackUrl, $originalFilename, $ownerId) { $CC_CONFIG = Config::getConfig(); From a07a1edcc0f52d1509e446c30ffa8ad36b288add Mon Sep 17 00:00:00 2001 From: Duncan Sommerville Date: Tue, 17 Feb 2015 12:17:49 -0500 Subject: [PATCH 2/3] SAAS-595 - Updated validation and sanitization --- airtime_mvc/application/Bootstrap.php | 1 + .../application/common/FileDataHelper.php | 20 ++++++++ .../controllers/LibraryController.php | 4 +- .../rest/controllers/MediaController.php | 48 +++++++++++++------ 4 files changed, 57 insertions(+), 16 deletions(-) create mode 100644 airtime_mvc/application/common/FileDataHelper.php diff --git a/airtime_mvc/application/Bootstrap.php b/airtime_mvc/application/Bootstrap.php index 2ebbac4ce..b4a4fb43a 100644 --- a/airtime_mvc/application/Bootstrap.php +++ b/airtime_mvc/application/Bootstrap.php @@ -11,6 +11,7 @@ require_once __DIR__."/configs/constants.php"; require_once 'Preference.php'; require_once 'Locale.php'; require_once "DateHelper.php"; +require_once "FileDataHelper.php"; require_once "HTTPHelper.php"; require_once "OsPath.php"; require_once "Database.php"; diff --git a/airtime_mvc/application/common/FileDataHelper.php b/airtime_mvc/application/common/FileDataHelper.php new file mode 100644 index 000000000..4f8738b05 --- /dev/null +++ b/airtime_mvc/application/common/FileDataHelper.php @@ -0,0 +1,20 @@ +id)) { - $objInfo = Application_Model_Library::getObjInfo($obj_sess->type); - $objInfo = Application_Model_Library::getObjInfo($obj_sess->type); $obj = new $objInfo['className']($obj_sess->id); $userInfo = Zend_Auth::getInstance()->getStorage()->read(); @@ -446,6 +444,8 @@ class LibraryController extends Zend_Controller_Action } if ($form->isValid($serialized)) { + // Sanitize any incorrect metadata that slipped past validation + FileDataHelper::sanitizeData($serialized["track_number"]); $formValues = $this->_getParam('data', null); $formdata = array(); diff --git a/airtime_mvc/application/modules/rest/controllers/MediaController.php b/airtime_mvc/application/modules/rest/controllers/MediaController.php index 1b9712bd2..74fc4c346 100644 --- a/airtime_mvc/application/modules/rest/controllers/MediaController.php +++ b/airtime_mvc/application/modules/rest/controllers/MediaController.php @@ -114,7 +114,8 @@ class Rest_MediaController extends Zend_Rest_Controller return; } else { // Sanitize any incorrect metadata that slipped past validation - $this->sanitizeData($file, $whiteList); + FileDataHelper::sanitizeData($whiteList["track_number"]); + /* If full_path is set, the post request came from ftp. * Users are allowed to upload folders via ftp. If this is the case * we need to include the folder name with the file name, otherwise @@ -166,6 +167,37 @@ class Rest_MediaController extends Zend_Rest_Controller if (!$this->validateRequestData($file, $whiteList)) { $file->save(); return; + } else if ($file && isset($requestData["resource_id"])) { + // Sanitize any incorrect metadata that slipped past validation + FileDataHelper::sanitizeData($whiteList["track_number"]); + + $file->fromArray($whiteList, BasePeer::TYPE_FIELDNAME); + + //store the original filename + $file->setDbFilepath($requestData["filename"]); + + $fileSizeBytes = $requestData["filesize"]; + if (!isset($fileSizeBytes) || $fileSizeBytes === false) + { + $file->setDbImportStatus(2)->save(); + $this->fileNotFoundResponse(); + return; + } + $cloudFile = new CloudFile(); + $cloudFile->setStorageBackend($requestData["storage_backend"]); + $cloudFile->setResourceId($requestData["resource_id"]); + $cloudFile->setCcFiles($file); + $cloudFile->save(); + + Application_Model_Preference::updateDiskUsage($fileSizeBytes); + + $now = new DateTime("now", new DateTimeZone("UTC")); + $file->setDbMtime($now); + $file->save(); + + $this->getResponse() + ->setHttpResponseCode(200) + ->appendBody(json_encode(CcFiles::sanitizeResponse($file))); } else if ($file) { // Sanitize any incorrect metadata that slipped past validation $this->sanitizeData($file, $whiteList); @@ -267,7 +299,7 @@ class Rest_MediaController extends Zend_Rest_Controller $fileForm = new Application_Form_EditAudioMD(); $fileForm->startForm($file->getDbId()); $fileForm->populate($whiteList); - + /* * Here we are truncating metadata of any characters greater than the * max string length set in the database. In the rare case a track's @@ -302,18 +334,6 @@ class Rest_MediaController extends Zend_Rest_Controller return true; } - /** - * We want to throw out invalid data and process the upload successfully - * at all costs, so check the whitelisted data and sanitize it if necessary - * @param CcFiles $file CcFiles object being uploaded - * @param array $whitelist array of whitelisted (modifiable) file fields - */ - private function sanitizeData($file, &$whitelist) { - if (!ctype_digit(strval($whitelist["track_number"]))) { - $file->setDbTrackNumber(null); - } - } - private function processUploadedFile($callbackUrl, $originalFilename, $ownerId) { $CC_CONFIG = Config::getConfig(); From 17f1d0e96deadc1e417bfd5e81e0102136ff8097 Mon Sep 17 00:00:00 2001 From: Albert Santoni Date: Wed, 18 Feb 2015 16:29:08 -0500 Subject: [PATCH 3/3] Simplify the metadata sanitization and bugfix it * SAAS-376 and CC-5868 --- .../application/common/FileDataHelper.php | 18 +++++++++------- .../controllers/LibraryController.php | 21 ++++--------------- .../rest/controllers/MediaController.php | 10 +++------ 3 files changed, 17 insertions(+), 32 deletions(-) diff --git a/airtime_mvc/application/common/FileDataHelper.php b/airtime_mvc/application/common/FileDataHelper.php index 4f8738b05..fc93c64fe 100644 --- a/airtime_mvc/application/common/FileDataHelper.php +++ b/airtime_mvc/application/common/FileDataHelper.php @@ -1,9 +1,4 @@ isValid($serialized)) { - // Sanitize any incorrect metadata that slipped past validation - FileDataHelper::sanitizeData($serialized["track_number"]); - - $formValues = $this->_getParam('data', null); - $formdata = array(); - foreach ($formValues as $val) { - $formdata[$val["name"]] = $val["value"]; - } - $file->setDbColMetadata($formdata); - - $data = $file->getMetadata(); - - // set MDATA_KEY_FILEPATH - $data['MDATA_KEY_FILEPATH'] = $file->getFilePath(); - Logging::info($data['MDATA_KEY_FILEPATH']); - Application_Model_RabbitMq::SendMessageToMediaMonitor("md_update", $data); - + $file->setDbColMetadata($serialized); $this->_redirect('Library'); } } diff --git a/airtime_mvc/application/modules/rest/controllers/MediaController.php b/airtime_mvc/application/modules/rest/controllers/MediaController.php index 74fc4c346..90587d480 100644 --- a/airtime_mvc/application/modules/rest/controllers/MediaController.php +++ b/airtime_mvc/application/modules/rest/controllers/MediaController.php @@ -113,9 +113,6 @@ class Rest_MediaController extends Zend_Rest_Controller $file->save(); return; } else { - // Sanitize any incorrect metadata that slipped past validation - FileDataHelper::sanitizeData($whiteList["track_number"]); - /* If full_path is set, the post request came from ftp. * Users are allowed to upload folders via ftp. If this is the case * we need to include the folder name with the file name, otherwise @@ -168,8 +165,6 @@ class Rest_MediaController extends Zend_Rest_Controller $file->save(); return; } else if ($file && isset($requestData["resource_id"])) { - // Sanitize any incorrect metadata that slipped past validation - FileDataHelper::sanitizeData($whiteList["track_number"]); $file->fromArray($whiteList, BasePeer::TYPE_FIELDNAME); @@ -199,8 +194,6 @@ class Rest_MediaController extends Zend_Rest_Controller ->setHttpResponseCode(200) ->appendBody(json_encode(CcFiles::sanitizeResponse($file))); } else if ($file) { - // Sanitize any incorrect metadata that slipped past validation - $this->sanitizeData($file, $whiteList); $file->fromArray($whiteList, BasePeer::TYPE_FIELDNAME); @@ -294,6 +287,9 @@ class Rest_MediaController extends Zend_Rest_Controller private function validateRequestData($file, &$whiteList) { + // Sanitize any wildly incorrect metadata before it goes to be validated + FileDataHelper::sanitizeData($whiteList); + try { // EditAudioMD form is used here for validation $fileForm = new Application_Form_EditAudioMD();