Security bug 1719491: Stop user saving bad first/last/preferred name

to avoid having problems when the user emails another user

Change-Id: If694e969ea8a9d77badfffa095dc5fac3b6da417
Signed-off-by: Robert Lyon <robertl@catalyst.net.nz>
(cherry picked from commit dbbbb91c)
(cherry picked from commit bf1717cb)

htdocs/artefact/internal/lib.php

@@ -506,11 +506,12 @@ class ArtefactTypeProfile extends ArtefactType {
     }
 
     public static function get_field_element_data() {
+        // we make sure the first/last/preferred names are safe as they get used in emails sent out
         return array(
-            'firstname'       => array('rules' => array('maxlength' => 50)),
-            'lastname'        => array('rules' => array('maxlength' => 50)),
+            'firstname'       => array('rules' => array('maxlength' => 50, 'safetext' => true)),
+            'lastname'        => array('rules' => array('maxlength' => 50, 'safetext' => true)),
             'studentid'       => array('rules' => array('maxlength' => 50)),
-            'preferredname'   => array('rules' => array('maxlength' => 50)),
+            'preferredname'   => array('rules' => array('maxlength' => 50, 'safetext' => true)),
         );
     }
 

htdocs/auth/lib.php

@@ -830,6 +830,7 @@ function auth_check_required_fields() {
     safe_require('artefact', 'internal');
 
     $alwaysmandatoryfields = array_keys(ArtefactTypeProfile::get_always_mandatory_fields());
+    $element_data = ArtefactTypeProfile::get_field_element_data();
     foreach(ArtefactTypeProfile::get_mandatory_fields() as $field => $type) {
         // Always mandatory fields are stored in the usr table, so are part of
         // the user session object. We can save a query by grabbing them from
@@ -864,6 +865,11 @@ function auth_check_required_fields() {
             'title' => get_string($field, 'artefact.internal'),
             'rules' => array('required' => true)
         );
+        // We need to merge the rules for the element if they have special rules defined
+        // in get_field_element_data() so that we save correct data.
+        if (isset($element_data[$field])) {
+            $elements[$field] = array_merge_recursive($elements[$field], $element_data[$field]);
+        }
 
         if ($field == 'socialprofile') {
             $elements[$field] = ArtefactTypeSocialprofile::get_new_profile_elements();
@@ -996,9 +1002,10 @@ function requiredfields_validate(Pieform $form, $values) {
             }
         }
     }
+
     // Check if email has been taken
     if (isset($values['email']) && record_exists('artefact_internal_profile_email', 'email', $values['email'])) {
-            $form->set_error('email', get_string('unvalidatedemailalreadytaken', 'artefact.internal'));
+        $form->set_error('email', get_string('unvalidatedemailalreadytaken', 'artefact.internal'));
     }
     // Check if the socialprofile url is valid.
     if (isset($values['socialprofile_hidden']) && $values['socialprofile_hidden'] && $values['socialprofile_profiletype'] == 'webpage' && !filter_var($values['socialprofile_profileurl'], FILTER_VALIDATE_URL)) {

htdocs/lang/en.utf8/pieforms.php

@@ -66,7 +66,7 @@ $string['rule.minvalue.minvalue'] = 'This value cannot be smaller than %d.';
 $string['rule.regex.regex'] = 'This field is not in valid form.';
 
 $string['rule.required.required'] = 'This field is required.';
-
+$string['rule.safetext.invalidchars'] = 'This field has invalid characters.';
 $string['rule.validateoptions.validateoptions'] = 'The option "%s" is invalid.';
 
 $string['rule.maxvalue.maxvalue'] = 'This value cannot be larger than %d.';

htdocs/lib/pieforms/pieform/rules/safetext.php

+<?php
+/**
+ * Pieforms: Advanced web forms made easy
+ * Copyright (C) 2006-2008 Catalyst IT Ltd (http://www.catalyst.net.nz)
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * @package    pieform
+ * @subpackage rule
+ * @author     Robert Lyon <robertl@catalyst.net.nz>
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL version 3 or later
+ * @copyright  For copyright information on Mahara, please see the README file distributed with this software.
+ *
+ */
+
+/**
+ * Checks whether the field has a safe text string.
+ *
+ * @param Pieform $form    The form the rule is being applied to
+ * @param string  $value   The value of the field
+ * @param array   $element The element to check
+ * @param string  $check   Whether to check the element
+ * @return string         The error message, if the value is invalid.
+ */
+function pieform_rule_safetext(Pieform $form, $value, $element, $check) {
+    if ($value != '') {
+        if ($value != strip_tags(clean_html($value))) {
+            return $form->i18n('rule', 'safetext', 'invalidchars', $element);
+        }
+    }
+}