MDL-70163 core: fix attachment validation when localrequestdir absent.

Switch the order of operations performed when collating list of paths
from which a user can include attachments.

First collect all normalised/absolute paths then filter empty entries,
which fixes an issue where $CFG->localrequestdir could be defined but
not exist. This would lead to an empty string being passed to strpos
which triggered a PHP warning.

Co-authored-by: Peter Burnett <peterburnett@catalyst-au.net>
This commit is contained in:
Paul Holden 2020-12-10 20:23:31 +00:00
parent fd840ab59c
commit 5ece7e75d7

View File

@ -6295,8 +6295,10 @@ function email_to_user($user, $from, $subject, $messagetext, $messagehtml = '',
// The absolute (real) path is also fetched to ensure that comparisons to allowed paths are compared equally.
$attachpath = str_replace('\\', '/', realpath($attachment));
// Add allowed paths to an array (also check if it's not empty).
$allowedpaths = array_filter([
// Build an array of all filepaths from which attachments can be added (normalised slashes, absolute/real path).
$allowedpaths = array_map(function(string $path): string {
return str_replace('\\', '/', realpath($path));
}, [
$CFG->cachedir,
$CFG->dataroot,
$CFG->dirroot,
@ -6304,12 +6306,12 @@ function email_to_user($user, $from, $subject, $messagetext, $messagehtml = '',
$CFG->tempdir,
$CFG->localrequestdir,
]);
// Set addpath to true.
$addpath = true;
// Check if attachment includes one of the allowed paths.
foreach ($allowedpaths as $allowedpath) {
// Make sure both variables are normalised before comparing.
$allowedpath = str_replace('\\', '/', realpath($allowedpath));
foreach (array_filter($allowedpaths) as $allowedpath) {
// Set addpath to false if the attachment includes one of the allowed paths.
if (strpos($attachpath, $allowedpath) === 0) {
$addpath = false;