From 5ece7e75d7ad21a0ffc50098d558da9bbe3df8a3 Mon Sep 17 00:00:00 2001
From: Paul Holden <paulh@moodle.com>
Date: Thu, 10 Dec 2020 20:23:31 +0000
Subject: [PATCH] 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>
---
 lib/moodlelib.php | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/lib/moodlelib.php b/lib/moodlelib.php
index 59fcd1c132f..626e7ad4fdc 100644
--- a/lib/moodlelib.php
+++ b/lib/moodlelib.php
@@ -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;