MDL-57531 mail: Validate the sender's email address
authorDavid Mudrák <david@moodle.com>
Tue, 3 Jan 2017 21:09:30 +0000 (22:09 +0100)
committerDavid Mudrák <david@moodle.com>
Wed, 4 Jan 2017 11:35:19 +0000 (12:35 +0100)
The patch adds validation for the noreplyaddress setting variable, for
the explicit $replyto parameter and for the sender's email. In case of
misconfigured noreplyaddress setting, it falls back to the default
noreply address value. In case of invalid email in the user's record,
the email is not sent.

The patch also adds unit test for the value returned by the function
generate_email_processing_address() so that it can be considered as a
valid email, too.

This is supposed to significantly minimise the risk of exploiting the
vulnerability in PHPMailer's Sender field.

lib/moodlelib.php
lib/tests/moodlelib_test.php
lib/tests/weblib_test.php

index f560d07..872bb7c 100644 (file)
@@ -5766,7 +5766,13 @@ function email_to_user($user, $from, $subject, $messagetext, $messagehtml = '',
     $tempreplyto = array();
 
     // Make sure that we fall back onto some reasonable no-reply address.
-    $noreplyaddress = empty($CFG->noreplyaddress) ? 'noreply@' . get_host_from_url($CFG->wwwroot) : $CFG->noreplyaddress;
+    $noreplyaddressdefault = 'noreply@' . get_host_from_url($CFG->wwwroot);
+    $noreplyaddress = empty($CFG->noreplyaddress) ? $noreplyaddressdefault : $CFG->noreplyaddress;
+
+    if (!validate_email($noreplyaddress)) {
+        debugging('email_to_user: Invalid noreply-email '.s($noreplyaddress));
+        $noreplyaddress = $noreplyaddressdefault;
+    }
 
     // Make up an email address for handling bounces.
     if (!empty($CFG->handlebounces)) {
@@ -5776,6 +5782,12 @@ function email_to_user($user, $from, $subject, $messagetext, $messagehtml = '',
         $mail->Sender = $noreplyaddress;
     }
 
+    // Make sure that the explicit replyto is valid, fall back to the implicit one.
+    if (!empty($replyto) && !validate_email($replyto)) {
+        debugging('email_to_user: Invalid replyto-email '.s($replyto));
+        $replyto = $noreplyaddress;
+    }
+
     $alloweddomains = null;
     if (!empty($CFG->allowedemaildomains)) {
         $alloweddomains = explode(PHP_EOL, $CFG->allowedemaildomains);
@@ -5793,6 +5805,11 @@ function email_to_user($user, $from, $subject, $messagetext, $messagehtml = '',
     // and that the senders email setting is either displayed to everyone, or display to only other users that are enrolled
     // in a course with the sender.
     } else if ($usetrueaddress && can_send_from_real_email_address($from, $user, $alloweddomains)) {
+        if (!validate_email($from->email)) {
+            debugging('email_to_user: Invalid from-email '.s($from->email).' - not sending');
+            // Better not to use $noreplyaddress in this case.
+            return false;
+        }
         $mail->From = $from->email;
         $fromdetails = new stdClass();
         $fromdetails->name = fullname($from);
index f7e7352..a65436e 100644 (file)
@@ -3425,4 +3425,27 @@ class core_moodlelib_testcase extends advanced_testcase {
              'samecourse' => false, 'result' => false],
         ];
     }
+
+    /**
+     * Test that generate_email_processing_address() returns valid email address.
+     */
+    public function test_generate_email_processing_address() {
+        global $CFG;
+        $this->resetAfterTest();
+
+        $data = (object)[
+            'id' => 42,
+            'email' => 'my.email+from_moodle@example.com',
+        ];
+
+        $modargs = 'B'.base64_encode(pack('V', $data->id)).substr(md5($data->email), 0, 16);
+
+        $CFG->maildomain = 'example.com';
+        $CFG->mailprefix = 'mdl+';
+        $this->assertEquals(1, validate_email(generate_email_processing_address(0, $modargs)));
+
+        $CFG->maildomain = 'mail.example.com';
+        $CFG->mailprefix = 'mdl-';
+        $this->assertEquals(1, validate_email(generate_email_processing_address(23, $modargs)));
+    }
 }
index 0d7faf6..c248b98 100644 (file)
@@ -665,4 +665,19 @@ EXPECTED;
         );
     }
 
+    /**
+     * Tests for validate_email() function.
+     */
+    public function test_validate_email() {
+
+        $this->assertEquals(1, validate_email('moodle@example.com'));
+        $this->assertEquals(1, validate_email('moodle@localhost.local'));
+        $this->assertEquals(1, validate_email('verp_email+is=mighty@moodle.org'));
+        $this->assertEquals(1, validate_email("but_potentially'dangerous'too@example.org"));
+        $this->assertEquals(1, validate_email('posts+AAAAAAAAAAIAAAAAAAAGQQAAAAABFSXz1eM/P/lR2bYyljM+@posts.moodle.org'));
+
+        $this->assertEquals(0, validate_email('moodle@localhost'));
+        $this->assertEquals(0, validate_email('"attacker\\" -oQ/tmp/ -X/var/www/vhost/moodle/backdoor.php  some"@email.com'));
+        $this->assertEquals(0, validate_email("moodle@example.com>\r\nRCPT TO:<victim@example.com"));
+    }
 }