Bugfix: CSRF outlined in CVE-2020-7988 (#3373)

Update app/tools/pass-change
 - Add CSRF cookie.
 - Require old password.
 - Prevent old password re-use.
 - Enforce password complexity requirements.

Fixes #3373

app/tools/pass-change/form.php

@@ -1,6 +1,9 @@
 <?php
 # user must be authenticated
 $User->check_user_session ();
+
+# create csrf token
+$csrf = $User->Crypto->csrf_cookie ("create", "pass-change");
 ?>
 
 <div class="col-xs-12 col-md-6 col-md-offset-3" style="margin-top:50px;">
@@ -20,15 +23,22 @@
 
 		<form name="changePassRequired" id="changePassRequiredForm" class="form-inline" method="post">
 		<div class="row" style="margin-top:30px;">
+			<input type="hidden" id="csrf_cookie" name="csrf_cookie" value="<?php print $csrf; ?>">
 
-			<!-- new password -->
-			<div class="col-xs-12 col-md-4"><strong><?php print _('Password'); ?></strong></div>
+			<!-- old password -->
+			<div class="col-xs-12 col-md-4"><strong><?php print _('Old Password'); ?></strong></div>
 			<div class="col-xs-12 col-md-8">
+				<input type="password" style="width:100%;" id="oldpassword" name="oldpassword" class="form-control" autofocus="autofocus" autocomplete="off" autocorrect="off" autocapitalize="off" spellcheck="false"></input>
+			</div>
+
+			<!-- new password -->
+			<div class="col-xs-12 col-md-4"  style="margin-top:30px;"><strong><?php print _('New Password'); ?></strong></div>
+			<div class="col-xs-12 col-md-8"  style="margin-top:30px;">
 				<input type="password" style="width:100%;" id="ipampassword1" name="ipampassword1" class="form-control" autofocus="autofocus" autocomplete="off" autocorrect="off" autocapitalize="off" spellcheck="false"></input>
 			</div>
 
 			<!-- new password repeat -->
-			<div class="col-xs-12 col-md-4" style="margin-top:10px;"><strong><?php print _('Password repeat'); ?></strong></div>
+			<div class="col-xs-12 col-md-4" style="margin-top:10px;"><strong><?php print _('New Password repeat'); ?></strong></div>
 			<div class="col-xs-12 col-md-8" style="margin-top:10px;">
 				<input type="password" style="width:100%;" id="ipampassword2" name="ipampassword2" class="form-control" autocomplete="off" autocorrect="off" autocapitalize="off" spellcheck="false"></input>
 			</div>

app/tools/pass-change/result.php

@@ -4,17 +4,33 @@
 require_once('../../../functions/functions.php');
 
 # Classes
-$Database 	= new Database_PDO;
-$User 	= new User ($Database);
+$Database = new Database_PDO;
+$User  = new User ($Database);
 $Result = new Result;
+$Password_check = new Password_check ();
 
 # user must be authenticated
 $User->check_user_session ();
 
-# checks
-if(strlen($_POST['ipampassword1'])<8)					{ $Result->show("danger", _("Invalid password"), true); }
-if($_POST['ipampassword1']!=$_POST['ipampassword2'])	{ $Result->show("danger", _("Passwords do not match"), true); }
+# Ensure keys exist (php8.0)
+$_POST = array_merge(array_fill_keys(['csrf_cookie', 'oldpassword', 'ipampassword1', 'ipampassword2'], null), $_POST);
+
+
+#CSRF
+$User->Crypto->csrf_cookie ("validate", "pass-change", $_POST['csrf_cookie']) === false ? $Result->show("danger", _("Invalid CSRF cookie"), true) : "";
+
+# Check old password
+if(!hash_equals($User->user->password, crypt($_POST['oldpassword'], $User->user->password))) { $Result->show("danger", _("Invalid password"), true); }
+
+# Check new password != old password
+if($_POST['ipampassword1']==$_POST['oldpassword']) { $Result->show("danger", _("New password must be different"), true); }
+
+# Enforce password policy
+$policy = (json_decode($User->settings->passwordPolicy, true));
+$Password_check->set_requirements($policy, explode(",",$policy['allowedSymbols']));
+if (!$Password_check->validate ($_POST['ipampassword1'])) { $Result->show("danger alert-danger ", _('Password validation errors').":<br> - ".implode("<br> - ", $Password_check->get_errors ()), true); }
+
+if($_POST['ipampassword1']!=$_POST['ipampassword2']) { $Result->show("danger", _("New passwords do not match"), true); }
 
 # update pass
 $User->update_user_pass($_POST['ipampassword1']);
-?>

functions/version.php

@@ -4,7 +4,7 @@
 /* set latest version */
 define("VERSION_VISIBLE", "1.4.4");							//visible version in footer e.g 1.3.2
 /* set latest revision */
-define("REVISION", "028");									//increment on static content changes (js/css) or point releases to avoid caching issues
+define("REVISION", "029");									//increment on static content changes (js/css) or point releases to avoid caching issues
 /* set last possible upgrade */
 define("LAST_POSSIBLE", "1.19");							//minimum required version to be able to upgrade
 /* set published - hide dbversion in footer */

js/magic.js

@@ -1183,13 +1183,8 @@ $('form#cform').submit(function () {
 /* changePassRequired */
 $('form#changePassRequiredForm').submit(function() {
 	showSpinner();
-
-    //get username
-    var ipampassword1 = $('#ipampassword1', this).val();
-    var ipampassword2 = $('#ipampassword2', this).val();
-    //get login data
-    var postData = "ipampassword1="+ipampassword1+"&ipampassword2="+ipampassword2;
-
+    //get csrf_cookie, old + new passwords
+    var postData = $('form#changePassRequiredForm').serialize();
     $.post('app/tools/pass-change/result.php', postData, function(data) {
         $('div#changePassRequiredResult').html(data).fadeIn('fast');
         hideSpinner();

misc/CHANGELOG

@@ -1,4 +1,5 @@
 == 1.4.4
+
     Bugfixes:
     ----------------------------
     + Allow UTF-8 in instruction widgets (#3360);
@@ -7,6 +8,7 @@
     Security Fixes:
     ----------------------------
     + XSS (reflected) in IP calculator (#3351);
+    + XSS in pass-change/result.php (#3373);
 
 == 1.4.3