L-electron-Rare/moodle
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

MDL-84010 core_files: sync_external_file can cause infinite recursion

Browse Source
This commit is contained in:
sam marshall
2025-08-05 07:49:02 +01:00
parent b479c7a056
commit a08193f094
2 changed files with 103 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lib/filestorage/tests/stored_file_test.php
+77
View File
@@ -117,4 +117,81 @@ final class stored_file_test extends advanced_testcase {
$stream->close();
}
/**
* If the data gets into an incorrect state where a file references itself, this should not
* get into endless recursion (stack overflow) but should throw an exception.
*/
public function test_sync_external_file_with_recursive_reference(): void {
global $DB;
$this->resetAfterTest();
$fs = get_file_storage();
$filerecord = [
'contextid' => context_system::instance()->id,
'component' => 'core',
'filearea' => 'unittest',
'itemid' => 0,
'filepath' => '/',
'filename' => 'hello.txt',
];
$file = $fs->create_file_from_string($filerecord, 'hello world');
$referenceid = $DB->get_field('repository_instances', 'id', ['typeid' => FILE_INTERNAL]);
$referencestr = \file_storage::pack_reference($filerecord);
$copyrecord = [
'contextid' => context_system::instance()->id,
'component' => 'core',
'filearea' => 'unittest',
'itemid' => 1,
'filepath' => '/',
'filename' => 'hello.txt',
];
$copy = $fs->create_file_from_reference($copyrecord, $referenceid, $referencestr);
// Hack the original file so that it has the reference id to itself from the copy.
$DB->set_field('files', 'referencefileid', $copy->get_referencefileid(), ['id' => $file->get_id()]);
// Now sync the original file.
$hackedfile = $fs->get_file_by_id($file->get_id());
try {
$hackedfile->sync_external_file();
$this->fail('Should not work because this is a recursive reference');
} catch (\moodle_exception $e) {
$this->assertStringContainsString('File references itself: ' . $file->get_id(), $e->getMessage());
}
// Create another file that references the copy.
$reference2str = \file_storage::pack_reference($copyrecord);
$copy2record = [
'contextid' => context_system::instance()->id,
'component' => 'core',
'filearea' => 'unittest',
'itemid' => 2,
'filepath' => '/',
'filename' => 'hello.txt',
];
$copy2 = $fs->create_file_from_reference($copy2record, $referenceid, $reference2str);
// Now we change the original file to reference this second one - 2 levels of redirection.
$DB->set_field('files', 'referencefileid', $copy2->get_referencefileid(), ['id' => $file->get_id()]);
// Again try to sync the original file.
$hackedfile = $fs->get_file_by_id($file->get_id());
try {
$hackedfile->sync_external_file();
$this->fail('Should not work because this is a recursive reference');
} catch (\moodle_exception $e) {
$this->assertStringContainsString('File references itself: ' . $file->get_id(), $e->getMessage());
}
// Put the hacked file back how it started so the situation is valid.
$DB->set_field('files', 'referencefileid', 0, ['id' => $file->get_id()]);
$copy2->sync_external_file();
$copy->sync_external_file();
$file->sync_external_file();
}
}
repository/lib.php
+26 -8
View File
@@ -536,6 +536,9 @@ abstract class repository implements cacheable_object {
/** @var bool true if the super construct is called, otherwise false. */
public $super_called;
/** @var array List of file ids currently being synced, to avoid endless recursion */
protected static $syncfileids = [];
/**
* Constructor
*
@@ -2759,14 +2762,29 @@ abstract class repository implements cacheable_object {
if ($file->get_referencelastsync()) {
return false;
}
$fs = get_file_storage();
$params = file_storage::unpack_reference($file->get_reference(), true);
if (!is_array($params) || !($storedfile = $fs->get_file($params['contextid'],
$params['component'], $params['filearea'], $params['itemid'], $params['filepath'],
$params['filename']))) {
$file->set_missingsource();
} else {
$file->set_synchronized($storedfile->get_contenthash(), $storedfile->get_filesize(), 0, $storedfile->get_timemodified());
if (in_array($file->get_id(), self::$syncfileids)) {
throw new \coding_exception('File references itself: ' . $file->get_id());
}
try {
array_push(self::$syncfileids, $file->get_id());
$fs = get_file_storage();
$params = file_storage::unpack_reference($file->get_reference(), true);
if (!is_array($params) || !($storedfile = $fs->get_file($params['contextid'],
$params['component'], $params['filearea'], $params['itemid'], $params['filepath'],
$params['filename']))) {
$file->set_missingsource();
} else {
$file->set_synchronized(
$storedfile->get_contenthash(),
$storedfile->get_filesize(),
0,
$storedfile->get_timemodified(),
);
}
} finally {
array_pop(self::$syncfileids);
}
return true;
}