Skip to content

Commit 5c4ef26

Browse files
marinaglancyDamyon Wiese
marinaglancy
authored and
Damyon Wiese
committed
MDL-45616 repositories: use json encoding instead of serialization
1 parent cb2b42a commit 5c4ef26

File tree

7 files changed

+49
-23
lines changed

7 files changed

+49
-23
lines changed

Diff for: ‎repository/coursefiles/lib.php

+5-5
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(),

Diff for: ‎repository/equella/callback.php

+1-1
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>

Diff for: ‎repository/equella/lib.php

+10-5
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,11 @@ public function supported_returntypes() {
119119
* @return string file referece
120120
*/
121121
public function get_file_reference($source) {
122-
return $source;
122+
// Internally we store serialized value but user input is json-encoded for security reasons.
123+
$ref = json_decode(base64_decode($source));
124+
$filename = clean_param($ref->filename, PARAM_FILE);
125+
$url = clean_param($ref->url, PARAM_URL);
126+
return base64_encode(serialize((object)array('url' => $url, 'filename' => $filename)));
123127
}
124128

125129
/**
@@ -407,12 +411,13 @@ private static function to_mime_type($value) {
407411
/**
408412
* Return the source information
409413
*
410-
* @param stdClass $url
414+
* @param string $source
411415
* @return string|null
412416
*/
413-
public function get_file_source_info($url) {
414-
$ref = unserialize(base64_decode($url));
415-
return 'EQUELLA: ' . $ref->filename;
417+
public function get_file_source_info($source) {
418+
$ref = json_decode(base64_decode($source));
419+
$filename = clean_param($ref->filename, PARAM_FILE);
420+
return 'EQUELLA: ' . $filename;
416421
}
417422

418423
/**

Diff for: ‎repository/lib.php

+23-3
Original file line numberDiff line numberDiff line change
@@ -1625,12 +1625,32 @@ public static function display_instances_list($context, $typename = null) {
16251625
* Prepare file reference information
16261626
*
16271627
* @param string $source source of the file, returned by repository as 'source' and received back from user (not cleaned)
1628-
* @return string file referece
1628+
* @return string file reference, ready to be stored
16291629
*/
16301630
public function get_file_reference($source) {
16311631
if ($source && $this->has_moodle_files()) {
1632-
$params = file_storage::unpack_reference($source);
1633-
if (!is_array($params)) {
1632+
$params = @json_decode(base64_decode($source), true);
1633+
if (!$params && !in_array($this->get_typename(), array('recent', 'user', 'local', 'coursefiles'))) {
1634+
// IMPORTANT! Since default format for moodle files was changed in the minor release as a security fix
1635+
// we maintain an old code here in order not to break 3rd party repositories that deal
1636+
// with moodle files. Repositories are strongly encouraged to be upgraded, see MDL-45616.
1637+
// In Moodle 2.8 this fallback will be removed.
1638+
$params = file_storage::unpack_reference($source, true);
1639+
return file_storage::pack_reference($params);
1640+
}
1641+
if (!is_array($params) || empty($params['contextid'])) {
1642+
throw new repository_exception('invalidparams', 'repository');
1643+
}
1644+
$params = array(
1645+
'component' => empty($params['component']) ? '' : clean_param($params['component'], PARAM_COMPONENT),
1646+
'filearea' => empty($params['filearea']) ? '' : clean_param($params['filearea'], PARAM_AREA),
1647+
'itemid' => empty($params['itemid']) ? 0 : clean_param($params['itemid'], PARAM_INT),
1648+
'filename' => empty($params['filename']) ? null : clean_param($params['filename'], PARAM_FILE),
1649+
'filepath' => empty($params['filepath']) ? null : clean_param($params['filepath'], PARAM_PATH),
1650+
'contextid' => clean_param($params['contextid'], PARAM_INT)
1651+
);
1652+
// Check if context exists.
1653+
if (!context::instance_by_id($params['contextid'], IGNORE_MISSING)) {
16341654
throw new repository_exception('invalidparams', 'repository');
16351655
}
16361656
return file_storage::pack_reference($params);

Diff for: ‎repository/local/lib.php

+3-3
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);
@@ -216,7 +216,7 @@ private function can_skip(file_info $fileinfo, $extensions, $parent = -1) {
216216
*/
217217
private function get_node(file_info $fileinfo) {
218218
global $OUTPUT;
219-
$encodedpath = base64_encode(serialize($fileinfo->get_params()));
219+
$encodedpath = base64_encode(json_encode($fileinfo->get_params()));
220220
$node = array(
221221
'title' => $fileinfo->get_visible_name(),
222222
'datemodified' => $fileinfo->get_timemodified(),
@@ -256,7 +256,7 @@ private function get_node(file_info $fileinfo) {
256256
* @return array
257257
*/
258258
private function get_node_path(file_info $fileinfo) {
259-
$encodedpath = base64_encode(serialize($fileinfo->get_params()));
259+
$encodedpath = base64_encode(json_encode($fileinfo->get_params()));
260260
return array(
261261
'path' => $encodedpath,
262262
'name' => $fileinfo->get_visible_name()

Diff for: ‎repository/recent/lib.php

+3-2
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(),
@@ -191,7 +191,8 @@ public function supported_returntypes() {
191191
*/
192192
public function file_is_accessible($source) {
193193
global $USER;
194-
$file = self::get_moodle_file($source);
194+
$reference = $this->get_file_reference($source);
195+
$file = self::get_moodle_file($reference);
195196
return (!empty($file) && $file->get_userid() == $USER->id);
196197
}
197198

Diff for: ‎repository/user/lib.php

+4-4
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)