View Issue Details

IDProjectCategoryView StatusLast Update
15852Bug reports[All Projects] Accessibilitypublic2020-02-20 09:00
Reporterffroehlich Assigned To 
PrioritynoneSeveritymajor 
Status newResolutionopen 
Product Version4.1.0 
Target VersionFixed in Version 
Summary15852: No login with long pssword
Description

I Installed the site with an admin password with 64 chars. The login field does only allow 40 chars. Changing html code (maxlength) passes the Validation
Maybe more versions are affected.

Additional Information

Remove maxlength from html code

TagsNo tags attached.
Complete LimeSurvey version number (& build)docker latest
I will donate to the project if issue is resolvedNo
Browser
Database & DB-Versionmysql
Server OS (if known)
Webserver software & version (if known)
PHP Versiondocker latest

Activities

TonisOrmisson

TonisOrmisson

2020-02-13 09:25

developer   ~55935

Im using >40 chars with no problems.
The only place in the code I can quickly find that limits the password size is the password "forgotpassword" form that limits the input to 64 chars

ffroehlich

ffroehlich

2020-02-13 10:19

reporter   ~55937

I checked out the current master branch and found at least at three places in the sourcecode hardcoded limitations of the password filed, two of them where limeted to 40, one to 240 chars. There is no reason why you limit the password field. Furthermore I haven't found a backend check for the length. I hope that other inputs are checked in the backend... I'll make a Pull Request at the weekend. Furthermore I see, that you are using old PHP functions and bad code. I'll write some more Bugtickets soon.

DenisChenu

DenisChenu

2020-02-14 16:51

developer   ~56023

@ffroehlich : usage of new Branch inside LimeSurvey repo is for LimeSurvey GMBH team. For external devloper : use your own repo and make pull request

Sample ; https://github.com/LimeSurvey/LimeSurvey/pull/1384

DenisChenu

DenisChenu

2020-02-14 16:52

developer   ~56024

@ollehar : You confirm about usage of new Branch inside LimeSurvey repo :) ?

ollehar

ollehar

2020-02-14 17:39

administrator   ~56027

Yes, branches are only accessible by employees at LimeSurvey GmbH. External devs should use PRs instead.

ffroehlich

ffroehlich

2020-02-14 20:03

reporter   ~56030

That is too complicated for me... Here is the git diff

Furthermore I found some critical vulnerabilities and I'll report them soon with a CVE number. Is there a way to report the Issues to you before they are published?



diff (4,133 bytes)
diff --git a/application/controllers/admin/statistics.php b/application/controllers/admin/statistics.php
index 0196eff666..e2d33049cf 100644
--- a/application/controllers/admin/statistics.php
+++ b/application/controllers/admin/statistics.php
@@ -156,9 +156,11 @@ class statistics extends Survey_Common_Action
          * b) "groups" -> group_name + group_order *
          */
 
-        //store all the data in $rows
-        $rows = Question::model()->with('group')->findAll(array('condition' => 'parent_qid = 0 AND group.sid=' . $surveyid, 'order' => 'group_order,question_order'));
 
+        $rows = Question::model()
+            ->with(array('group' => array('alias' => 'g')))
+            ->findAll(array('condition' => 'parent_qid = 0 AND g.sid=' . $surveyid, 'order' => 'group_order,question_order'));
+      
         //SORT IN NATURAL ORDER!
         usort($rows, 'groupOrderThenQuestionOrder');
 
diff --git a/application/core/plugins/AuthLDAP/AuthLDAP.php b/application/core/plugins/AuthLDAP/AuthLDAP.php
index a1b217934d..08f5ec6e4c 100644
--- a/application/core/plugins/AuthLDAP/AuthLDAP.php
+++ b/application/core/plugins/AuthLDAP/AuthLDAP.php
@@ -363,7 +363,7 @@ class AuthLDAP extends LimeSurvey\PluginManager\AuthPluginBase
     {
         $this->getEvent()->getContent($this)
         ->addContent(CHtml::tag('span', array(), "<label for='user'>".gT("Username")."</label>".CHtml::textField('user', '', array('size'=>40, 'maxlength'=>40, 'class'=>"form-control"))))
-        ->addContent(CHtml::tag('span', array(), "<label for='password'>".gT("Password")."</label>".CHtml::passwordField('password', '', array('size'=>40, 'maxlength'=>40, 'class'=>"form-control"))));
+        ->addContent(CHtml::tag('span', array(), "<label for='password'>".gT("Password")."</label>".CHtml::passwordField('password', '', array('size'=>40, 'class'=>"form-control"))));
     }
 
     /**
diff --git a/application/core/plugins/Authdb/Authdb.php b/application/core/plugins/Authdb/Authdb.php
index 2d94fb5928..9137da28fa 100644
--- a/application/core/plugins/Authdb/Authdb.php
+++ b/application/core/plugins/Authdb/Authdb.php
@@ -122,7 +122,7 @@ class Authdb extends AuthPluginBase
 
         $this->getEvent()->getContent($this)
                 ->addContent(CHtml::tag('span', array(), "<label for='user'>".gT("Username")."</label>".CHtml::textField('user', $sUserName, array('size'=>240, 'maxlength'=>240, 'class'=>"form-control"))))
-                ->addContent(CHtml::tag('span', array(), "<label for='password'>".gT("Password")."</label>".CHtml::passwordField('password', $sPassword, array('size'=>240, 'maxlength'=>240, 'class'=>"form-control"))));
+                ->addContent(CHtml::tag('span', array(), "<label for='password'>".gT("Password")."</label>".CHtml::passwordField('password', $sPassword, array('size'=>240, 'class'=>"form-control"))));
     }
 
     public function newUserSession()
diff --git a/application/extensions/bootstrap/helpers/TbHtml.php b/application/extensions/bootstrap/helpers/TbHtml.php
index 1c927a3404..cc5498711b 100755
--- a/application/extensions/bootstrap/helpers/TbHtml.php
+++ b/application/extensions/bootstrap/helpers/TbHtml.php
@@ -862,6 +862,12 @@ class TbHtml extends CHtml // required in order to access the protected methods
      */
     public static function passwordField($name, $value = '', $htmlOptions = array())
     {
+
+        if (array_key_exists('maxlength', $htmlOptions)) {
+
+            trigger_error('maxlength is not supported for password fields', E_USER_NOTICE);
+            unset ($htmlOptions['maxlength']);
+        }
         return self::textInputField('password', $name, $value, $htmlOptions);
     }
 
@@ -1799,6 +1805,11 @@ EOD;
      */
     public static function activePasswordField($model, $attribute, $htmlOptions = array())
     {
+        if (array_key_exists('maxlength', $htmlOptions)) {
+
+            trigger_error('maxlength is not supported for password fields', E_USER_NOTICE);
+            unset ($htmlOptions['maxlength']);
+        }
         return self::activeTextInputField('password', $model, $attribute, $htmlOptions);
     }
 
diff (4,133 bytes)
ffroehlich

ffroehlich

2020-02-19 22:47

reporter   ~56101

Any Response?

DenisChenu

DenisChenu

2020-02-20 09:00

developer   ~56105

In yout diff : i see only the LDAP update. All other are not related and not needed

Issue History

Date Modified Username Field Change
2020-02-12 12:23 ffroehlich New Issue
2020-02-13 09:25 TonisOrmisson Note Added: 55935
2020-02-13 10:19 ffroehlich Note Added: 55937
2020-02-14 16:51 DenisChenu Note Added: 56023
2020-02-14 16:52 DenisChenu Note Added: 56024
2020-02-14 17:39 ollehar Note Added: 56027
2020-02-14 20:03 ffroehlich File Added: diff
2020-02-14 20:03 ffroehlich Note Added: 56030
2020-02-19 22:47 ffroehlich Note Added: 56101
2020-02-20 09:00 DenisChenu Note Added: 56105