Skip to content

Commit 04483c5

Browse files
Modernizes password strength checks by using zxcvbn
Signed-off-by: Jon Stovell <jonstovell@gmail.com>
1 parent d382804 commit 04483c5

40 files changed

Lines changed: 2867 additions & 51 deletions

.php-cs-fixer.dist.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
'Sources/minify',
2222
'Sources/random_compat',
2323
'Sources/ReCaptcha',
24+
'Sources/ZxcvbnPhp',
2425
'Themes',
2526
])
2627
// Skip all index.php files and ssi_example.php.

Languages/en_US/Errors.php

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -373,12 +373,41 @@
373373
$txt['profile_error_bad_avatar_url_too_long'] = 'The avatar URL you specified is too long, please use a shorter URL.';
374374
$txt['profile_error_bad_avatar_too_large'] = 'The image you are trying to use surpasses the max width/height settings, please use a smaller one.';
375375
$txt['profile_error_bad_avatar_fail_reencode'] = 'The image you uploaded was corrupted and the attempt to recover it failed.';
376+
376377
$txt['profile_error_password_short'] = 'Your password must be {0, plural,
377378
one {at least # character long}
378379
other {at least # characters long}
379380
}.';
380-
$txt['profile_error_password_restricted_words'] = 'Your password must not contain your username, email address or other commonly used words.';
381-
$txt['profile_error_password_chars'] = 'Your password must contain a mix of upper and lower case letters, as well as digits.';
381+
$txt['profile_error_password_restricted_words'] = 'Your password must not contain your username, email address, or other commonly used words.';
382+
$txt['profile_error_password_weak'] = 'This password is too weak.';
383+
$txt['profile_error_password_top_10'] = 'This is a top-10 common password.';
384+
$txt['profile_error_password_top_100'] = 'This is a top-100 common password.';
385+
$txt['profile_error_password_very_common'] = 'This is a very common password.';
386+
$txt['profile_error_password_similar_to_common'] = 'This is similar to a commonly used password.';
387+
$txt['profile_error_password_single_word'] = 'A word by itself is easy to guess.';
388+
$txt['profile_error_password_use_a_few_words'] = 'Use a few words, avoid common phrases.';
389+
$txt['profile_error_password_simple_is_fine'] = 'No need for symbols, digits, or uppercase letters.';
390+
$txt['profile_error_password_add_more_words'] = 'Add another word or two. Uncommon words are better.';
391+
$txt['profile_error_password_recent_years'] = 'Recent years are easy to guess.';
392+
$txt['profile_error_password_avoid_recent_years'] = 'Avoid recent years.';
393+
$txt['profile_error_password_avoid_personal_years'] = 'Avoid years that are associated with you.';
394+
$txt['profile_error_password_dates_are_easy'] = 'Dates are often easy to guess.';
395+
$txt['profile_error_password_avoid_personal_dates_and_years'] = 'Avoid dates and years that are associated with you.';
396+
$txt['profile_error_password_straight_rows'] = 'Straight rows of keys are easy to guess.';
397+
$txt['profile_error_password_short_patterns'] = 'Short keyboard patterns are easy to guess.';
398+
$txt['profile_error_password_use_longer_pattern'] = 'Use a longer keyboard pattern with more turns.';
399+
$txt['profile_error_password_sequences'] = 'Sequences like “abc” or “6543” are easy to guess.';
400+
$txt['profile_error_password_avoid_sequences'] = 'Avoid sequences.';
401+
$txt['profile_error_password_reversed_words'] = 'Reversed words aren’t much harder to guess.';
402+
$txt['profile_error_password_repeated_chars'] = 'Repeats like “aaa” are easy to guess.';
403+
$txt['profile_error_password_repeated_strings'] = 'Repeats like “abcabcabc” are only slightly harder to guess than “abc”.';
404+
$txt['profile_error_password_avoid_repeated'] = 'Avoid repeated words and characters.';
405+
$txt['profile_error_password_l33t_useless'] = 'Predictable substitutions like “@” instead of “a” don’t help very much.';
406+
$txt['profile_error_password_names'] = 'Names and surnames by themselves are easy to guess.';
407+
$txt['profile_error_password_common_names'] = 'Common names and surnames are easy to guess.';
408+
$txt['profile_error_password_caps_useless'] = 'Capitalization doesn’t help very much.';
409+
$txt['profile_error_password_all_caps_useless'] = 'All uppercase is almost as easy to guess as all lowercase.';
410+
382411
$txt['profile_error_already_requested_group'] = 'You already have an outstanding request for this group!';
383412
$txt['profile_error_signature_not_yet_saved'] = 'The signature has not been saved.';
384413
$txt['profile_error_personal_text_too_long'] = 'The personal text is too long.';

Languages/en_US/Help.php

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -466,12 +466,13 @@
466466
$helptxt['approveAccountDeletion'] = 'When this setting is checked, any user request to delete his own account has to be approved by an administrator';
467467

468468
$helptxt['send_welcomeEmail'] = 'When this setting is enabled all new members will be sent an email welcoming them to your community';
469-
$helptxt['password_strength'] = 'This setting determines the strength required for passwords selected by your forum users. The stronger the password, the harder it should be to compromise member’s accounts.
470-
Its possible settings are:
469+
$helptxt['password_strength'] = 'This setting determines the strength required for passwords selected by your forum users. The stronger the password, the harder it should be to compromise member’s accounts. Passwords are evaluated using the <a href="https://github.com/bjeavons/zxcvbn-php" class="bbc_link" target="_blank" rel="noopener">zxcvbn library</a> and checked for other requirements.
470+
<br><br>
471+
Possible values are:
471472
<ul class="normallist">
472-
<li><strong>Low:</strong> The password must be at least four characters long.</li>
473-
<li><strong>Medium:</strong> The password must be at least eight characters long, and cannot be part of a user’s name or email address.</li>
474-
<li><strong>High:</strong> As for medium, except the password must also contain a mixture of upper and lower case letters, and at least one number.</li>
473+
<li><strong>Low:</strong> The password must be at least six characters long, it must not contain the user’s name or email address, and it must earn a zxcvbn score of at least 2.</li>
474+
<li><strong>Medium:</strong> As for low, except the password must be at least eight characters long and it must earn a zxcvbn score of at least 3.</li>
475+
<li><strong>High:</strong> As for medium, except it must earn a zxcvbn score of 4.</li>
475476
</ul>';
476477
$helptxt['enable_password_conversion'] = 'By enabling this setting, SMF will attempt to detect passwords stored in other formats and convert them to the format SMF uses. Typically this is used for forums converted to SMF, but may have other uses as well. Disabling this prevents a user from logging in using their password after a conversion and they would need to reset their password.';
477478

Languages/en_US/ManageSettings.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -168,9 +168,9 @@
168168
$txt['loadavg_disabled_conf'] = 'Load balancing support is disabled by your host configuration.';
169169

170170
$txt['setting_password_strength'] = 'Required strength for user passwords';
171-
$txt['setting_password_strength_low'] = 'Low - 4 character minimum';
172-
$txt['setting_password_strength_medium'] = 'Medium - cannot contain username';
173-
$txt['setting_password_strength_high'] = 'High - mixture of different characters';
171+
$txt['setting_password_strength_low'] = 'Low';
172+
$txt['setting_password_strength_medium'] = 'Medium';
173+
$txt['setting_password_strength_high'] = 'High';
174174
$txt['setting_enable_password_conversion'] = 'Allow password hash conversion';
175175

176176
$txt['antispam_Settings'] = 'Anti-Spam Verification';

Languages/en_US/Profile.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@
4747
$txt['dob_month'] = 'Month (MM)';
4848
$txt['dob_day'] = 'Day (DD)';
4949
$txt['dob_year'] = 'Year (YYYY)';
50-
$txt['password_strength'] = 'For best security, you should use eight or more characters with a combination of letters, numbers, and symbols.';
50+
$txt['password_strength'] = 'For best security, you should use eight or more characters. It is a good idea to use a phrase with multiple words.';
5151
$txt['include_website_url'] = 'This must be included if you specify a URL below.';
5252
$txt['complete_url'] = 'This must be a complete URL.';
5353
$txt['sig_info'] = 'Signatures are displayed at the bottom of each post or personal message. BBCode and smileys may be used in your signature.';

Sources/Actions/Register2.php

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -518,10 +518,16 @@ public static function registerMember(array &$reg_options, bool $return_errors =
518518

519519
// Password isn't legal?
520520
if ($password_error != null) {
521-
$error_code = ['lang', 'profile_error_password_' . $password_error, false];
521+
Lang::load('Errors');
522+
523+
if (isset(Lang::$txt['profile_error_password_' . $password_error])) {
524+
$error_code = ['lang', 'profile_error_password_' . $password_error, false];
525+
} else {
526+
$error_code = ['done', $password_error, false];
527+
}
522528

523529
if ($password_error == 'short') {
524-
$error_code[] = [empty(Config::$modSettings['password_strength']) ? 4 : 8];
530+
$error_code[] = Security::minimumPasswordLength();
525531
}
526532

527533
$reg_errors[] = $error_code;

Sources/Actions/Reminder.php

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -242,14 +242,16 @@ public function setPassword2(): void
242242
$this->loadMember();
243243

244244
// Is the password actually valid?
245-
$passwordError = Security::validatePassword($_POST['passwrd1'], $this->member->username, [$this->member->email]);
245+
$password_error = Security::validatePassword($_POST['passwrd1'], $this->member->username, [$this->member->email]);
246246

247247
// What - it's not?
248-
if ($passwordError != null) {
249-
if ($passwordError == 'short') {
250-
ErrorHandler::fatalLang('profile_error_password_' . $passwordError, false, [empty(Config::$modSettings['password_strength']) ? 4 : 8]);
248+
if ($password_error != null) {
249+
if ($password_error == 'short') {
250+
ErrorHandler::fatalLang('profile_error_password_short', false, [Security::minimumPasswordLength()]);
251251
} else {
252-
ErrorHandler::fatalLang('profile_error_password_' . $passwordError, false);
252+
Lang::load('Errors');
253+
254+
ErrorHandler::fatalLang((isset(Lang::$txt['profile_error_password_' . $password_error]) ? 'profile_error_password_' : '') . $password_error, false);
253255
}
254256
}
255257

@@ -380,14 +382,16 @@ public function secretAnswer2(): void
380382
}
381383

382384
// Make sure they have a strong enough password.
383-
$passwordError = Security::validatePassword($_POST['passwrd1'], $this->member->username, [$this->member->email]);
385+
$password_error = Security::validatePassword($_POST['passwrd1'], $this->member->username, [$this->member->email]);
384386

385387
// Invalid?
386-
if ($passwordError != null) {
387-
if ($passwordError == 'short') {
388-
ErrorHandler::fatalLang('profile_error_password_' . $passwordError, false, [empty(Config::$modSettings['password_strength']) ? 4 : 8]);
388+
if ($password_error != null) {
389+
if ($password_error == 'short') {
390+
ErrorHandler::fatalLang('profile_error_password_' . $password_error, false, [Security::minimumPasswordLength()]);
389391
} else {
390-
ErrorHandler::fatalLang('profile_error_password_' . $passwordError, false);
392+
Lang::load('Errors');
393+
394+
ErrorHandler::fatalLang((isset(Lang::$txt['profile_error_password_' . $password_error]) ? 'profile_error_password_' : '') . $password_error, false);
391395
}
392396
}
393397

Sources/Autoloader.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
'ReCaptcha\\' => 'ReCaptcha/',
2929
'MatthiasMullie\\Minify\\' => 'minify/src/',
3030
'MatthiasMullie\\PathConverter\\' => 'minify/path-converter/src/',
31+
'ZxcvbnPhp\\' => 'ZxcvbnPhp/',
3132

3233
// In general, the SMF namespace maps to $sourcedir.
3334
'SMF\\' => '',

Sources/Profile.php

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -605,11 +605,13 @@ public function loadStandardFields(bool $force_reload = false)
605605
}
606606

607607
// Let's get the validation function into play...
608-
$passwordErrors = Security::validatePassword(Utils::htmlspecialcharsDecode($value), $this->username, [$this->name, User::$me->username, User::$me->name, User::$me->email]);
608+
$password_error = Security::validatePassword(Utils::htmlspecialcharsDecode($value), $this->username, [$this->name, User::$me->username, User::$me->name, User::$me->email]);
609609

610610
// Were there errors?
611-
if ($passwordErrors != null) {
612-
return 'password_' . $passwordErrors;
611+
if ($password_error != null) {
612+
Lang::load('Errors');
613+
614+
return (isset(Lang::$txt['profile_error_password_' . $password_error]) ? 'password_' : '') . $password_error;
613615
}
614616

615617
// Set up the new password variable... ready for storage.

Sources/Security.php

Lines changed: 77 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
use SMF\Cache\CacheApi;
1919
use SMF\Db\DatabaseApi as Db;
20+
use ZxcvbnPhp\Zxcvbn;
2021

2122
/**
2223
* A collection of miscellaneous methods related to forum security.
@@ -98,6 +99,16 @@ public static function generateValidationCode(): string
9899
return bin2hex(random_bytes(5));
99100
}
100101

102+
/**
103+
* Gets the minimum required password length.
104+
*
105+
* @return int The minimum required password length.
106+
*/
107+
public static function minimumPasswordLength(): int
108+
{
109+
return empty(Config::$modSettings['password_strength']) ? 6 : 8;
110+
}
111+
101112
/**
102113
* Checks whether a password meets the current forum rules.
103114
*
@@ -117,7 +128,7 @@ public static function generateValidationCode(): string
117128
public static function validatePassword(string $password, string $username, array $restrict_in = []): ?string
118129
{
119130
// Perform basic requirements first.
120-
if (Utils::entityStrlen($password) < (empty(Config::$modSettings['password_strength']) ? 4 : 8)) {
131+
if (Utils::entityStrlen($password) < self::minimumPasswordLength()) {
121132
return 'short';
122133
}
123134

@@ -130,37 +141,79 @@ public static function validatePassword(string $password, string $username, arra
130141
return $pass_error;
131142
}
132143

133-
// Is this enough?
134-
if (empty(Config::$modSettings['password_strength'])) {
135-
return null;
136-
}
137-
138-
// Otherwise, perform the medium strength test - checking if password appears in the restricted string.
139-
if (preg_match('~\b' . preg_quote($password, '~') . '\b~', implode(' ', $restrict_in))) {
144+
if (
145+
// Check if password appears in the restricted strings.
146+
preg_match('~\b' . preg_quote($password, '~') . '\b~u', implode(' ', $restrict_in))
147+
// Check if password appears in the username.
148+
|| preg_match('~\b' . preg_quote($password, '~') . '\b~iu', $username)
149+
// Check if username appears in the password.
150+
|| preg_match('~\b' . preg_quote($username, '~') . '\b~iu', $password)
151+
) {
140152
return 'restricted_words';
141153
}
142154

143-
if (Utils::entityStrpos($password, $username) !== false) {
144-
return 'restricted_words';
145-
}
155+
// Use zxcvbn to assess the password's strength.
156+
$zxcvbn = new Zxcvbn();
157+
$strength = $zxcvbn->passwordStrength($password, array_merge([$username], $restrict_in));
146158

147-
// If just medium, we're done.
148-
if (Config::$modSettings['password_strength'] == 1) {
149-
return null;
150-
}
159+
if ((int) $strength['score'] < (Config::$modSettings['password_strength'] ?? 0) + 2) {
160+
Lang::load('Errors');
151161

152-
// Check for both numbers and letters.
153-
$good = preg_match('~\p{N}~u', $password) && preg_match('~\p{L}~u', $password);
162+
// List of known feedback strings from zxcvbn mapped to Lang::$txt keys.
163+
$feedback_strings = [
164+
'This is a top-10 common password' => 'top_10',
165+
'This is a top-100 common password' => 'top_100',
166+
'This is a very common password' => 'very_common',
167+
'This is similar to a commonly used password' => 'similar_to_common',
168+
'A word by itself is easy to guess' => 'single_word',
169+
'Use a few words, avoid common phrases' => 'use_a_few_words',
170+
'No need for symbols, digits, or uppercase letters' => 'simple_is_fine',
171+
'Add another word or two. Uncommon words are better.' => 'add_more_words',
172+
'Recent years are easy to guess' => 'recent_years',
173+
'Avoid recent years' => 'avoid_recent_years',
174+
'Avoid years that are associated with you' => 'avoid_personal_years',
175+
'Dates are often easy to guess' => 'dates_are_easy',
176+
'Avoid dates and years that are associated with you' => 'avoid_personal_dates_and_years',
177+
'Straight rows of keys are easy to guess' => 'straight_rows',
178+
'Short keyboard patterns are easy to guess' => 'short_patterns',
179+
'Use a longer keyboard pattern with more turns' => 'use_longer_pattern',
180+
'Sequences like abc or 6543 are easy to guess' => 'sequences',
181+
'Avoid sequences' => 'avoid_sequences',
182+
'Reversed words aren\'t much harder to guess' => 'reversed_words',
183+
'Repeats like "aaa" are easy to guess' => 'repeated_chars',
184+
'Repeats like "abcabcabc" are only slightly harder to guess than "abc"' => 'repeated_strings',
185+
'Avoid repeated words and characters' => 'avoid_repeated',
186+
'Predictable substitutions like \'@\' instead of \'a\' don\'t help very much' => 'l33t_useless',
187+
'Names and surnames by themselves are easy to guess' => 'names',
188+
'Common names and surnames are easy to guess' => 'common_names',
189+
'Capitalization doesn\'t help very much' => 'caps_useless',
190+
'All-uppercase is almost as easy to guess as all-lowercase' => 'all_caps_useless',
191+
];
192+
193+
$feedback = [];
194+
195+
if (isset($strength['feedback']['warning'], $feedback_strings[$strength['feedback']['warning']])) {
196+
$feedback[] = Lang::getTxt('profile_error_password_' . $feedback_strings[$strength['feedback']['warning']]);
197+
}
154198

155-
// If there are any letters from bicameral scripts (Latin, Greek, etc.),
156-
// check that there are both lowercase and uppercase letters present.
157-
// Note: If the password only contains letters from a unicameral script
158-
// (Arabic, Thai, etc.), this requirement is not applicable.
159-
if (Utils::strtoupper($password) !== ($lower_password = Utils::strtolower($password))) {
160-
$good &= $password !== $lower_password;
199+
if (!empty($strength['feedback']['suggestions'])) {
200+
foreach ($strength['feedback']['suggestions'] as $suggestion) {
201+
if (isset($feedback_strings[$suggestion])) {
202+
$feedback[] = Lang::getTxt('profile_error_password_' . $feedback_strings[$suggestion]);
203+
}
204+
}
205+
}
206+
207+
if (!empty($feedback)) {
208+
return implode('<br>', $feedback);
209+
}
210+
211+
// Generic error message.
212+
return 'weak';
161213
}
162214

163-
return $good ? null : 'chars';
215+
// If we get here, the password is strong enough.
216+
return null;
164217
}
165218

166219
/**

0 commit comments

Comments
 (0)