Permalink
Browse files

MDL-33950 check if source file is accessible in repository_ajax.php

- repository::copy_to_area() does not check access any more, and repository_recent::copy_to_area() is unnecessary
- added repository::file_is_accessible() that checks access to the picked file (regardless of accessibility of the file it is referencing to)
  • Loading branch information...
1 parent b2e5a60 commit db8152585056317d5ec42b3e21b838d89c1ef9ad @marinaglancy marinaglancy committed with danpoltawski Jun 28, 2012
Showing with 66 additions and 121 deletions.
  1. +45 −38 repository/lib.php
  2. +12 −81 repository/recent/lib.php
  3. +9 −2 repository/repository_ajax.php
View
@@ -651,45 +651,60 @@ public static function draftfile_exists($itemid, $filepath, $filename) {
* @return stored_file|null
*/
public static function get_moodle_file($source) {
- $params = unserialize(base64_decode($source));
- if (empty($params) || !is_array($params)) {
- return null;
- }
- foreach (array('contextid', 'itemid', 'filename', 'filepath', 'component') as $key) {
- if (!array_key_exists($key, $params)) {
- return null;
+ $params = file_storage::unpack_reference($source, true);
+ $fs = get_file_storage();
+ return $fs->get_file($params['contextid'], $params['component'], $params['filearea'],
+ $params['itemid'], $params['filepath'], $params['filename']);
+ }
+
+ /**
+ * Repository method to make sure that user can access particular file.
+ *
+ * This is checked when user tries to pick the file from repository to deal with
+ * potential parameter substitutions is request
+ *
+ * @param string $source
+ * @return bool whether the file is accessible by current user
+ */
+ public function file_is_accessible($source) {
+ if ($this->has_moodle_files()) {
+ try {
+ $params = file_storage::unpack_reference($source, true);
+ } catch (file_reference_exception $e) {
+ return false;
}
+ $browser = get_file_browser();
+ $context = get_context_instance_by_id($params['contextid']);
+ $file_info = $browser->get_file_info($context, $params['component'], $params['filearea'],
+ $params['itemid'], $params['filepath'], $params['filename']);
+ return !empty($file_info);
}
- $contextid = clean_param($params['contextid'], PARAM_INT);
- $component = clean_param($params['component'], PARAM_COMPONENT);
- $filearea = clean_param($params['filearea'], PARAM_AREA);
- $itemid = clean_param($params['itemid'], PARAM_INT);
- $filepath = clean_param($params['filepath'], PARAM_PATH);
- $filename = clean_param($params['filename'], PARAM_FILE);
- $fs = get_file_storage();
- return $fs->get_file($contextid, $component, $filearea, $itemid, $filepath, $filename);
+ return true;
}
/**
- * This function is used to copy a moodle file to draft area
+ * This function is used to copy a moodle file to draft area.
+ *
+ * It DOES NOT check if the user is allowed to access this file because the actual file
+ * can be located in the area where user does not have access to but there is an alias
+ * to this file in the area where user CAN access it.
+ * {@link file_is_accessible} should be called for alias location before calling this function.
*
- * @param string $encoded The metainfo of file, it is base64 encoded php serialized data
+ * @param string $source The metainfo of file, it is base64 encoded php serialized data
* @param stdClass|array $filerecord contains itemid, filepath, filename and optionally other
* attributes of the new file
* @param int $maxbytes maximum allowed size of file, -1 if unlimited. If size of file exceeds
* the limit, the file_exception is thrown.
- * @return array The information of file
+ * @return array The information about the created file
*/
- public function copy_to_area($encoded, $filerecord, $maxbytes = -1) {
+ public function copy_to_area($source, $filerecord, $maxbytes = -1) {
global $USER;
$fs = get_file_storage();
- $browser = get_file_browser();
if ($this->has_moodle_files() == false) {
throw new coding_exception('Only repository used to browse moodle files can use repository::copy_to_area()');
}
- $params = unserialize(base64_decode($encoded));
$user_context = context_user::instance($USER->id);
$filerecord = (array)$filerecord;
@@ -701,25 +716,17 @@ public function copy_to_area($encoded, $filerecord, $maxbytes = -1) {
$new_filepath = $filerecord['filepath'];
$new_filename = $filerecord['filename'];
- $contextid = clean_param($params['contextid'], PARAM_INT);
- $fileitemid = clean_param($params['itemid'], PARAM_INT);
- $filename = clean_param($params['filename'], PARAM_FILE);
- $filepath = clean_param($params['filepath'], PARAM_PATH);;
- $filearea = clean_param($params['filearea'], PARAM_AREA);
- $component = clean_param($params['component'], PARAM_COMPONENT);
-
- $context = get_context_instance_by_id($contextid);
// the file needs to copied to draft area
- $file_info = $browser->get_file_info($context, $component, $filearea, $fileitemid, $filepath, $filename);
- if ($maxbytes !== -1 && $file_info->get_filesize() > $maxbytes) {
+ $stored_file = self::get_moodle_file($source);
+ if ($maxbytes != -1 && $stored_file->get_filesize() > $maxbytes) {
throw new file_exception('maxbytes');
}
if (repository::draftfile_exists($draftitemid, $new_filepath, $new_filename)) {
// create new file
$unused_filename = repository::get_unused_filename($draftitemid, $new_filepath, $new_filename);
$filerecord['filename'] = $unused_filename;
- $file_info->copy_to_storage($filerecord);
+ $fs->create_file_from_storedfile($filerecord, $stored_file);
$event = array();
$event['event'] = 'fileexists';
$event['newfile'] = new stdClass;
@@ -729,17 +736,17 @@ public function copy_to_area($encoded, $filerecord, $maxbytes = -1) {
$event['existingfile'] = new stdClass;
$event['existingfile']->filepath = $new_filepath;
$event['existingfile']->filename = $new_filename;
- $event['existingfile']->url = moodle_url::make_draftfile_url($draftitemid, $new_filepath, $new_filename)->out();;
+ $event['existingfile']->url = moodle_url::make_draftfile_url($draftitemid, $new_filepath, $new_filename)->out();
return $event;
} else {
- $file_info->copy_to_storage($filerecord);
+ $fs->create_file_from_storedfile($filerecord, $stored_file);
$info = array();
$info['itemid'] = $draftitemid;
- $info['file'] = $new_filename;
- $info['title'] = $new_filename;
+ $info['file'] = $new_filename;
+ $info['title'] = $new_filename;
$info['contextid'] = $user_context->id;
- $info['url'] = moodle_url::make_draftfile_url($draftitemid, $new_filepath, $new_filename)->out();;
- $info['filesize'] = $file_info->get_filesize();
+ $info['url'] = moodle_url::make_draftfile_url($draftitemid, $new_filepath, $new_filename)->out();
+ $info['filesize'] = $stored_file->get_filesize();
return $info;
}
}
@@ -177,91 +177,22 @@ public static function type_config_form($mform, $classname = 'repository') {
public function supported_returntypes() {
return FILE_INTERNAL;
}
+
/**
- * This function overwrite the default implement to copying file using file_storage
+ * Repository method to make sure that user can access particular file.
+ *
+ * This is checked when user tries to pick the file from repository to deal with
+ * potential parameter substitutions is request
*
- * @param string $encoded The information of file, it is base64 encoded php serialized data
- * @param stdClass|array $filerecord contains itemid, filepath, filename and optionally other
- * attributes of the new file
- * @param int $maxbytes maximum allowed size of file, -1 if unlimited. If size of file exceeds
- * the limit, the file_exception is thrown.
- * @return array The information of file
+ * @todo MDL-33805 remove this function when recent files are managed correctly
+ *
+ * @param string $source
+ * @return bool whether the file is accessible by current user
*/
- public function copy_to_area($encoded, $filerecord, $maxbytes = -1) {
+ public function file_is_accessible($source) {
global $USER;
-
- $user_context = get_context_instance(CONTEXT_USER, $USER->id);
-
- $filerecord = (array)$filerecord;
- // make sure the new file will be created in user draft area
- $filerecord['component'] = 'user'; // make sure
- $filerecord['filearea'] = 'draft'; // make sure
- $filerecord['contextid'] = $user_context->id;
- $filerecord['sortorder'] = 0;
- $draftitemid = $filerecord['itemid'];
- $new_filepath = $filerecord['filepath'];
- $new_filename = $filerecord['filename'];
-
- $fs = get_file_storage();
-
- $params = unserialize(base64_decode($encoded));
-
- $contextid = clean_param($params['contextid'], PARAM_INT);
- $fileitemid = clean_param($params['itemid'], PARAM_INT);
- $filename = clean_param($params['filename'], PARAM_FILE);
- $filepath = clean_param($params['filepath'], PARAM_PATH);;
- $filearea = clean_param($params['filearea'], PARAM_AREA);
- $component = clean_param($params['component'], PARAM_COMPONENT);
-
- // XXX:
- // When user try to pick a file from other filearea, normally file api will use file browse to
- // operate the files with capability check, but in some areas, users don't have permission to
- // browse the files (for example, forum_attachment area).
- //
- // To get 'recent' plugin working, we need to use lower level file_stoarge class to bypass the
- // capability check, we will use a better workaround to improve it.
- // TODO MDL-33297 apply here
- if ($stored_file = $fs->get_file($contextid, $component, $filearea, $fileitemid, $filepath, $filename)) {
- // verify user id
- if ($USER->id != $stored_file->get_userid()) {
- throw new moodle_exception('errornotyourfile', 'repository');
- }
- if ($maxbytes !== -1 && $stored_file->get_filesize() > $maxbytes) {
- throw new file_exception('maxbytes');
- }
-
- // test if file already exists
- if (repository::draftfile_exists($draftitemid, $new_filepath, $new_filename)) {
- // create new file
- $unused_filename = repository::get_unused_filename($draftitemid, $new_filepath, $new_filename);
- $filerecord['filename'] = $unused_filename;
- // create a tmp file
- $fs->create_file_from_storedfile($filerecord, $stored_file);
- $event = array();
- $event['event'] = 'fileexists';
- $event['newfile'] = new stdClass;
- $event['newfile']->filepath = $new_filepath;
- $event['newfile']->filename = $unused_filename;
- $event['newfile']->url = moodle_url::make_draftfile_url($draftitemid, $new_filepath, $unused_filename)->out();
- $event['existingfile'] = new stdClass;
- $event['existingfile']->filepath = $new_filepath;
- $event['existingfile']->filename = $new_filename;
- $event['existingfile']->url = moodle_url::make_draftfile_url($draftitemid, $new_filepath, $new_filename)->out();;
- return $event;
- } else {
- $fs->create_file_from_storedfile($filerecord, $stored_file);
- $info = array();
- $info['title'] = $new_filename;
- $info['file'] = $new_filename;
- $info['itemid'] = $draftitemid;
- $info['filesize'] = $stored_file->get_filesize();
- $info['url'] = moodle_url::make_draftfile_url($draftitemid, $new_filepath, $new_filename)->out();;
- $info['contextid'] = $user_context->id;
- return $info;
- }
- }
- return false;
-
+ $file = self::get_moodle_file($source);
+ return (!empty($file) && $file->get_userid() == $USER->id);
}
/**
@@ -232,7 +232,14 @@
$record->userid = $USER->id;
$record->sortorder = 0;
+ // Check that user has permission to access this file
+ if (!$repo->file_is_accessible($source)) {
+ throw new file_exception('storedfilecannotread');
+ }
+
// If file is already a reference, set $source = file source, $repo = file repository
+ // note that in this case user may not have permission to access the source file directly
+ // so no file_browser/file_info can be used below
if ($repo->has_moodle_files()) {
$file = repository::get_moodle_file($source);
if ($file && $file->is_external_file()) {
@@ -298,13 +305,13 @@
} else {
// Download file to moodle.
$downloadedfile = $repo->get_file($source, $saveas_filename);
- if ($downloadedfile['path'] === false) {
+ if (empty($downloadedfile['path'])) {
$err->error = get_string('cannotdownload', 'repository');
die(json_encode($err));
}
// Check if exceed maxbytes.
- if (($maxbytes!==-1) && (filesize($downloadedfile['path']) > $maxbytes)) {
+ if ($maxbytes != -1 && filesize($downloadedfile['path']) > $maxbytes) {
throw new file_exception('maxbytes');
}

0 comments on commit db81525

Please sign in to comment.