Skip to content

Commit 7bcf9b1

Browse files
marinaglancyDamyon Wiese
marinaglancy
authored and
Damyon Wiese
committed
MDL-45616 repositories: use json encoding instead of serialization
1 parent 3fe1059 commit 7bcf9b1

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
@@ -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
/**
@@ -414,12 +418,13 @@ private static function to_mime_type($value) {
414418
/**
415419
* Return the source information
416420
*
417-
* @param stdClass $url
421+
* @param string $source
418422
* @return string|null
419423
*/
420-
public function get_file_source_info($url) {
421-
$ref = unserialize(base64_decode($url));
422-
return 'EQUELLA: ' . $ref->filename;
424+
public function get_file_source_info($source) {
425+
$ref = json_decode(base64_decode($source));
426+
$filename = clean_param($ref->filename, PARAM_FILE);
427+
return 'EQUELLA: ' . $filename;
423428
}
424429

425430
/**

Diff for: ‎repository/lib.php

+23-3
Original file line numberDiff line numberDiff line change
@@ -1691,12 +1691,32 @@ public static function display_instances_list($context, $typename = null) {
16911691
* Prepare file reference information
16921692
*
16931693
* @param string $source source of the file, returned by repository as 'source' and received back from user (not cleaned)
1694-
* @return string file referece
1694+
* @return string file reference, ready to be stored
16951695
*/
16961696
public function get_file_reference($source) {
16971697
if ($source && $this->has_moodle_files()) {
1698-
$params = file_storage::unpack_reference($source);
1699-
if (!is_array($params)) {
1698+
$params = @json_decode(base64_decode($source), true);
1699+
if (!$params && !in_array($this->get_typename(), array('recent', 'user', 'local', 'coursefiles'))) {
1700+
// IMPORTANT! Since default format for moodle files was changed in the minor release as a security fix
1701+
// we maintain an old code here in order not to break 3rd party repositories that deal
1702+
// with moodle files. Repositories are strongly encouraged to be upgraded, see MDL-45616.
1703+
// In Moodle 2.8 this fallback will be removed.
1704+
$params = file_storage::unpack_reference($source, true);
1705+
return file_storage::pack_reference($params);
1706+
}
1707+
if (!is_array($params) || empty($params['contextid'])) {
1708+
throw new repository_exception('invalidparams', 'repository');
1709+
}
1710+
$params = array(
1711+
'component' => empty($params['component']) ? '' : clean_param($params['component'], PARAM_COMPONENT),
1712+
'filearea' => empty($params['filearea']) ? '' : clean_param($params['filearea'], PARAM_AREA),
1713+
'itemid' => empty($params['itemid']) ? 0 : clean_param($params['itemid'], PARAM_INT),
1714+
'filename' => empty($params['filename']) ? null : clean_param($params['filename'], PARAM_FILE),
1715+
'filepath' => empty($params['filepath']) ? null : clean_param($params['filepath'], PARAM_PATH),
1716+
'contextid' => clean_param($params['contextid'], PARAM_INT)
1717+
);
1718+
// Check if context exists.
1719+
if (!context::instance_by_id($params['contextid'], IGNORE_MISSING)) {
17001720
throw new repository_exception('invalidparams', 'repository');
17011721
}
17021722
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(),
@@ -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

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)