View Issue Details

This bug affects 1 person(s).
 4
IDProjectCategoryView StatusLast Update
08175Bug reportsOtherpublic2013-10-10 19:26
Reporterabezverkhyy Assigned Toc_schmitz  
PrioritynormalSeverityminor 
Status closedResolutionfixed 
Product Version2.00+ 
Fixed in Version2.00+ 
Summary08175: In LimeSurvey administration preferences e-mail addresses are never validated before saving them
Description

In LimeSurvey administration preferences e-mail addresses are never validated before saving :

  • general settings : administrator's e-mail
  • general settings : administrator's bounce e-mail
  • new survey form : administrator's e-mail
  • new survey form : administrator's bounce e-mail
  • edit survey settings : administrator's e-mail
  • edit survey settings : administrator's bounce e-mail

as the result, the application will save obviously invalid e-mails like "foo bar" and try to send e-mail to them.

Steps To Reproduce

Change administrator's email to "foo bar". See your mail.log.

Additional Information

I wrote a simple patch that validates e-mails using validateEmailAddress and prevents obviously malformed e-mail from being written into LimeSurvey settings.

TagsNo tags attached.
Attached Files
limesurvey200plus-build130913_lng_2012101010000055.diff (3,642 bytes)   
--- limesurvey.orig/application/controllers/admin/database.php	2013-09-13 21:44:11.000000000 +0200
+++ limesurvey/application/controllers/admin/database.php	2013-09-16 17:56:46.271960511 +0200
@@ -987,6 +987,20 @@
 
             //make sure only numbers are passed within the $_POST variable
             $tokenlength = (int) $_POST['tokenlength'];
+	   
+            // Validate email addresses 
+            if ( ! validateEmailAddress($_POST['adminemail']))
+            {
+                Yii::app()->session['flashmessage'] = $clang->gT("Survey could not be updated because notification email is not valid.");
+                $this->getController()->redirect($this->getController()->createUrl('admin/survey/sa/editsurveysettings/surveyid/'.$surveyid));
+                return;
+            }
+            if ( ! validateEmailAddress($_POST['bounce_email']))
+            {
+                Yii::app()->session['flashmessage'] = $clang->gT("Survey could not be updated because bounce email is not valid.");
+                $this->getController()->redirect($this->getController()->createUrl('admin/survey/sa/editsurveysettings/surveyid/'.$surveyid));
+                return;
+            }
 
             //token length has to be at least 5, otherwise set it to default (15)
             if($tokenlength < 5)
--- limesurvey.orig/application/controllers/admin/globalsettings.php	2013-09-13 21:44:11.000000000 +0200
+++ limesurvey/application/controllers/admin/globalsettings.php	2013-09-16 18:00:26.939956930 +0200
@@ -123,6 +123,19 @@
         $clang = $this->getController()->lang;
         Yii::app()->loadHelper('surveytranslator');
 
+        // Validate email addresses
+        if( ! validateEmailAddress($_POST['siteadminemail']))
+        {
+                Yii::app()->session['flashmessage'] = $clang->gT("Site admin email is not valid.");
+                return;
+        }
+        if( ! validateEmailAddress($_POST['siteadminbounce']))
+        {
+                Yii::app()->session['flashmessage'] = $clang->gT("Site admin bounce email in not valid.");
+                return;
+        }
+
+
         $maxemails = $_POST['maxemails'];
         if (sanitize_int($_POST['maxemails']) < 1) {
             $maxemails = 1;
--- limesurvey.orig/application/controllers/admin/surveyadmin.php	2013-09-13 21:44:11.000000000 +0200
+++ limesurvey/application/controllers/admin/surveyadmin.php	2013-09-16 17:59:57.371957410 +0200
@@ -1509,6 +1509,20 @@
                 return;
             }
 
+            // Validate emails 
+            if ( ! validateEmailAddress($_POST['adminemail']))
+            {
+                Yii::app()->session['flashmessage'] = $this->getController()->lang->gT("Survey could not be created because notification email is not valid.");
+                $this->getController()->redirect($this->getController()->createUrl('admin/survey/sa/newsurvey'));
+                return;
+            }
+            if ( ! validateEmailAddress($_POST['bounce_email']))
+            {
+                Yii::app()->session['flashmessage'] = $this->getController()->lang->gT("Survey could not be created because bounce email is not valid.");
+                $this->getController()->redirect($this->getController()->createUrl('admin/survey/sa/newsurvey'));
+                return;
+            }
+
             // Check if template may be used
             $sTemplate = $_POST['template'];
             if (!$sTemplate || (Yii::app()->session['USER_RIGHT_SUPERADMIN'] != 1 && Yii::app()->session['USER_RIGHT_MANAGE_TEMPLATE'] != 1 && !hasTemplateManageRights(Yii::app()->session['loginID'], $_POST['template'])))
Bug heat4
Complete LimeSurvey version number (& build)130913
I will donate to the project if issue is resolvedNo
Browser
Database type & versionMySQL 5.5.31+dfsg-1
Server OS (if known)Debian GNU/Linux unstable
Webserver software & version (if known)Apache 2.4.6-3
PHP VersionPHP 5.5.3+dfsg-1

Relationships

related to 08256 closed Fatal error: Can't use method return value in write context in ...\application\controllers\admin\surveyadmin.php on line 1557 

Users monitoring this issue

There are no users monitoring this issue.

Activities

c_schmitz

c_schmitz

2013-09-27 17:30

administrator   ~26403

First let me say thank you! for taking the effort to create this patch. However the patch has several issues.

  • The bounce email is not mandatory, neither is the administration email. However it would be good style to give the administrator a hint that the entered address(es) was/were invalid using a flashmessage.
  • If the form is submitted then with your current solution all data is lost if the admin email is invalid. It should still save all remaining data, but maybe clearing out the offending email field.

If possible please submit the new patch as a pull request on gitHub - that way we will be able to handle your patch much quicker and can give you feedback accordingly.

abezverkhyy

abezverkhyy

2013-10-02 11:20

reporter   ~26471

Hello,

I changed my patch as follows : when forms are submitted, emails are checked, if valid they are saved, otherwise a warning is issued in the flash message and the rest of the form is saved as usual.

I made a pull request as you asked :
https://github.com/LimeSurvey/LimeSurvey/pull/137

c_schmitz

c_schmitz

2013-10-09 11:22

administrator   ~26657

2.00+ Build 121009 released

c_schmitz

c_schmitz

2013-10-09 18:05

administrator   ~26678

Fix committed to 2.05 branch: http://bugs.limesurvey.org/plugin.php?page=Source/view&amp;id=13210

Related Changesets

LimeSurvey: 2.05 e55ea645

2013-10-02 09:06:59

user6464


Committer: c_schmitz Details Diff
Fixed issue 08175: In administration preferences e-mail addresses are never validated before saving them Affected Issues
08175
mod - application/controllers/admin/database.php Diff File
mod - application/controllers/admin/globalsettings.php Diff File
mod - application/controllers/admin/surveyadmin.php Diff File

LimeSurvey: master 5602515b

2013-10-07 14:30:53

user6379

Details Diff
Merge pull request #137 from Grapsus/email_validation

08175 fix : in admin, check emails before saving them to database
Affected Issues
08175
mod - application/controllers/admin/database.php Diff File
mod - application/controllers/admin/globalsettings.php Diff File
mod - application/controllers/admin/surveyadmin.php Diff File

Issue History

Date Modified Username Field Change
2013-09-19 19:07 abezverkhyy New Issue
2013-09-19 19:07 abezverkhyy File Added: limesurvey200plus-build130913_lng_2012101010000055.diff
2013-09-27 17:01 c_schmitz Assigned To => c_schmitz
2013-09-27 17:01 c_schmitz Status new => assigned
2013-09-27 17:30 c_schmitz Note Added: 26403
2013-09-27 17:30 c_schmitz Status assigned => feedback
2013-10-02 11:20 abezverkhyy Note Added: 26471
2013-10-02 11:20 abezverkhyy Status feedback => assigned
2013-10-07 21:51 c_schmitz Changeset attached => LimeSurvey master 5602515b
2013-10-07 21:52 c_schmitz Status assigned => resolved
2013-10-07 21:52 c_schmitz Fixed in Version => 2.00+
2013-10-07 21:52 c_schmitz Resolution open => fixed
2013-10-09 11:22 c_schmitz Note Added: 26657
2013-10-09 11:22 c_schmitz Status resolved => closed
2013-10-09 18:05 c_schmitz Changeset attached => LimeSurvey 2.05 e55ea645
2013-10-09 18:05 c_schmitz Note Added: 26678
2013-10-10 19:26 mfaber Relationship added related to 08256