Skip to content

Commit 6196144

Browse files
marinaglancyDamyon Wiese
marinaglancy
authored and
Damyon Wiese
committed
MDL-45616 repositories: use json encoding instead of serialization
1 parent 867f409 commit 6196144

File tree

7 files changed

+49
-23
lines changed

7 files changed

+49
-23
lines changed

‎repository/coursefiles/lib.php‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ public function get_listing($encodedpath = '', $page = '') {
6363
$browser = get_file_browser();
6464

6565
if (!empty($encodedpath)) {
66-
$params = unserialize(base64_decode($encodedpath));
66+
$params = json_decode(base64_decode($encodedpath), true);
6767
if (is_array($params)) {
6868
$filepath = is_null($params['filepath']) ? NULL : clean_param($params['filepath'], PARAM_PATH);
6969
$filename = is_null($params['filename']) ? NULL : clean_param($params['filename'], PARAM_FILE);
@@ -80,12 +80,12 @@ public function get_listing($encodedpath = '', $page = '') {
8080
if ($fileinfo = $browser->get_file_info($context, $component, $filearea, $itemid, $filepath, $filename)) {
8181
// build path navigation
8282
$pathnodes = array();
83-
$encodedpath = base64_encode(serialize($fileinfo->get_params()));
83+
$encodedpath = base64_encode(json_encode($fileinfo->get_params()));
8484
$pathnodes[] = array('name'=>$fileinfo->get_visible_name(), 'path'=>$encodedpath);
8585
$level = $fileinfo->get_parent();
8686
while ($level) {
8787
$params = $level->get_params();
88-
$encodedpath = base64_encode(serialize($params));
88+
$encodedpath = base64_encode(json_encode($params));
8989
if ($params['contextid'] != $context->id) {
9090
break;
9191
}
@@ -102,7 +102,7 @@ public function get_listing($encodedpath = '', $page = '') {
102102
if ($child->is_directory()) {
103103
$params = $child->get_params();
104104
$subdir_children = $child->get_children();
105-
$encodedpath = base64_encode(serialize($params));
105+
$encodedpath = base64_encode(json_encode($params));
106106
$node = array(
107107
'title' => $child->get_visible_name(),
108108
'datemodified' => $child->get_timemodified(),
@@ -113,7 +113,7 @@ public function get_listing($encodedpath = '', $page = '') {
113113
);
114114
$list[] = $node;
115115
} else {
116-
$encodedpath = base64_encode(serialize($child->get_params()));
116+
$encodedpath = base64_encode(json_encode($child->get_params()));
117117
$node = array(
118118
'title' => $child->get_visible_name(),
119119
'size' => $child->get_filesize(),

‎repository/equella/callback.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@
5555
$license = s(clean_param($info->license, PARAM_ALPHAEXT));
5656
}
5757

58-
$source = base64_encode(serialize((object)array('url'=>$url,'filename'=>$filename)));
58+
$source = base64_encode(json_encode(array('url'=>$url,'filename'=>$filename)));
5959

6060
$js =<<<EOD
6161
<html>

‎repository/equella/lib.php‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,11 @@ public function supported_returntypes() {
125125
* @return string file referece
126126
*/
127127
public function get_file_reference($source) {
128-
return $source;
128+
// Internally we store serialized value but user input is json-encoded for security reasons.
129+
$ref = json_decode(base64_decode($source));
130+
$filename = clean_param($ref->filename, PARAM_FILE);
131+
$url = clean_param($ref->url, PARAM_URL);
132+
return base64_encode(serialize((object)array('url' => $url, 'filename' => $filename)));
129133
}
130134

131135
/**
@@ -400,12 +404,13 @@ private static function to_mime_type($value) {
400404
/**
401405
* Return the source information
402406
*
403-
* @param stdClass $url
407+
* @param string $source
404408
* @return string|null
405409
*/
406-
public function get_file_source_info($url) {
407-
$ref = unserialize(base64_decode($url));
408-
return 'EQUELLA: ' . $ref->filename;
410+
public function get_file_source_info($source) {
411+
$ref = json_decode(base64_decode($source));
412+
$filename = clean_param($ref->filename, PARAM_FILE);
413+
return 'EQUELLA: ' . $filename;
409414
}
410415

411416
/**

‎repository/lib.php‎

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1666,12 +1666,32 @@ public static function display_instances_list($context, $typename = null) {
16661666
* Prepare file reference information
16671667
*
16681668
* @param string $source source of the file, returned by repository as 'source' and received back from user (not cleaned)
1669-
* @return string file referece
1669+
* @return string file reference, ready to be stored
16701670
*/
16711671
public function get_file_reference($source) {
16721672
if ($source && $this->has_moodle_files()) {
1673-
$params = file_storage::unpack_reference($source);
1674-
if (!is_array($params)) {
1673+
$params = @json_decode(base64_decode($source), true);
1674+
if (!$params && !in_array($this->get_typename(), array('recent', 'user', 'local', 'coursefiles'))) {
1675+
// IMPORTANT! Since default format for moodle files was changed in the minor release as a security fix
1676+
// we maintain an old code here in order not to break 3rd party repositories that deal
1677+
// with moodle files. Repositories are strongly encouraged to be upgraded, see MDL-45616.
1678+
// In Moodle 2.8 this fallback will be removed.
1679+
$params = file_storage::unpack_reference($source, true);
1680+
return file_storage::pack_reference($params);
1681+
}
1682+
if (!is_array($params) || empty($params['contextid'])) {
1683+
throw new repository_exception('invalidparams', 'repository');
1684+
}
1685+
$params = array(
1686+
'component' => empty($params['component']) ? '' : clean_param($params['component'], PARAM_COMPONENT),
1687+
'filearea' => empty($params['filearea']) ? '' : clean_param($params['filearea'], PARAM_AREA),
1688+
'itemid' => empty($params['itemid']) ? 0 : clean_param($params['itemid'], PARAM_INT),
1689+
'filename' => empty($params['filename']) ? null : clean_param($params['filename'], PARAM_FILE),
1690+
'filepath' => empty($params['filepath']) ? null : clean_param($params['filepath'], PARAM_PATH),
1691+
'contextid' => clean_param($params['contextid'], PARAM_INT)
1692+
);
1693+
// Check if context exists.
1694+
if (!context::instance_by_id($params['contextid'], IGNORE_MISSING)) {
16751695
throw new repository_exception('invalidparams', 'repository');
16761696
}
16771697
return file_storage::pack_reference($params);

‎repository/local/lib.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ public function get_listing($encodedpath = '', $page = '') {
5656
$component = null;
5757

5858
if (!empty($encodedpath)) {
59-
$params = unserialize(base64_decode($encodedpath));
59+
$params = json_decode(base64_decode($encodedpath), true);
6060
if (is_array($params) && isset($params['contextid'])) {
6161
$component = is_null($params['component']) ? NULL : clean_param($params['component'], PARAM_COMPONENT);
6262
$filearea = is_null($params['filearea']) ? NULL : clean_param($params['filearea'], PARAM_AREA);
@@ -205,7 +205,7 @@ private function can_skip(file_info $fileinfo, $extensions, $parent = -1) {
205205
*/
206206
private function get_node(file_info $fileinfo) {
207207
global $OUTPUT;
208-
$encodedpath = base64_encode(serialize($fileinfo->get_params()));
208+
$encodedpath = base64_encode(json_encode($fileinfo->get_params()));
209209
$node = array(
210210
'title' => $fileinfo->get_visible_name(),
211211
'datemodified' => $fileinfo->get_timemodified(),
@@ -245,7 +245,7 @@ private function get_node(file_info $fileinfo) {
245245
* @return array
246246
*/
247247
private function get_node_path(file_info $fileinfo) {
248-
$encodedpath = base64_encode(serialize($fileinfo->get_params()));
248+
$encodedpath = base64_encode(json_encode($fileinfo->get_params()));
249249
return array(
250250
'path' => $encodedpath,
251251
'name' => $fileinfo->get_visible_name()

‎repository/recent/lib.php‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ public function get_listing($encodedpath = '', $page = '') {
126126
$fileinfo = $browser->get_file_info($context, $file['component'],
127127
$file['filearea'], $file['itemid'], $file['filepath'], $file['filename']);
128128
if ($fileinfo) {
129-
$params = base64_encode(serialize($file));
129+
$params = base64_encode(json_encode($file));
130130
$node = array(
131131
'title' => $fileinfo->get_visible_name(),
132132
'size' => $fileinfo->get_filesize(),
@@ -192,7 +192,8 @@ public function supported_returntypes() {
192192
*/
193193
public function file_is_accessible($source) {
194194
global $USER;
195-
$file = self::get_moodle_file($source);
195+
$reference = $this->get_file_reference($source);
196+
$file = self::get_moodle_file($reference);
196197
return (!empty($file) && $file->get_userid() == $USER->id);
197198
}
198199

‎repository/user/lib.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ public function get_listing($encodedpath = '', $page = '') {
6161
$list = array();
6262

6363
if (!empty($encodedpath)) {
64-
$params = unserialize(base64_decode($encodedpath));
64+
$params = json_decode(base64_decode($encodedpath), true);
6565
if (is_array($params)) {
6666
$filepath = clean_param($params['filepath'], PARAM_PATH);
6767
$filename = clean_param($params['filename'], PARAM_FILE);
@@ -84,7 +84,7 @@ public function get_listing($encodedpath = '', $page = '') {
8484
$level = $fileinfo;
8585
$params = $fileinfo->get_params();
8686
while ($level && $params['component'] == 'user' && $params['filearea'] == 'private') {
87-
$encodedpath = base64_encode(serialize($level->get_params()));
87+
$encodedpath = base64_encode(json_encode($level->get_params()));
8888
$pathnodes[] = array('name'=>$level->get_visible_name(), 'path'=>$encodedpath);
8989
$level = $level->get_parent();
9090
$params = $level->get_params();
@@ -95,7 +95,7 @@ public function get_listing($encodedpath = '', $page = '') {
9595
$children = $fileinfo->get_children();
9696
foreach ($children as $child) {
9797
if ($child->is_directory()) {
98-
$encodedpath = base64_encode(serialize($child->get_params()));
98+
$encodedpath = base64_encode(json_encode($child->get_params()));
9999
$node = array(
100100
'title' => $child->get_visible_name(),
101101
'datemodified' => $child->get_timemodified(),
@@ -106,7 +106,7 @@ public function get_listing($encodedpath = '', $page = '') {
106106
);
107107
$list[] = $node;
108108
} else {
109-
$encodedpath = base64_encode(serialize($child->get_params()));
109+
$encodedpath = base64_encode(json_encode($child->get_params()));
110110
$node = array(
111111
'title' => $child->get_visible_name(),
112112
'size' => $child->get_filesize(),

0 commit comments

Comments
 (0)