WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 12 months ago

Last modified 7 months ago

#31647 closed enhancement (fixed)

zxcvbn.js is old

Reported by: muranyia Owned by: pento
Milestone: 4.8 Priority: normal
Severity: normal Version:
Component: External Libraries Keywords:
Focuses: javascript Cc:

Description

wp-includes/js/zxcvbn.min.js is version 1.0 and rather old.
I don't know about other issues but it contains an invalid ("blah-blah") dictionary. For example:

o("male_names",r("wnzrf,wbua,eboreg,zvpunry,jvyyvnz,qnivq,evpuneq,puneyrf,..."))

See https://github.com/dropbox/zxcvbn/blob/master/zxcvbn.js for an up-to-date version.

Attachments (1)

31647.diff (2.4 KB) - added by pento 12 months ago.

Download all attachments as: .zip

Change History (14)

#1 @SergeyBiryukov
3 years ago

  • Component changed from Users to External Libraries

#2 in reply to: ↑ description @ocean90
3 years ago

Replying to muranyia:

I don't know about other issues but it contains an invalid ("blah-blah") dictionary.

We are using ROT13 for the word dictionaries see [25156].

The only change seems to be https://github.com/dropbox/zxcvbn/commit/482ed03a10779ec125100721c2d828b97abf9ea6.

#3 @iseulde
3 years ago

  • Version trunk deleted

#4 @swissspidy
2 years ago

There were quite some changes to the library recently. The current version is 3.5.0.

#5 @muranyia
14 months ago

  • Type changed from defect (bug) to enhancement
  • Version set to 4.6.1

As of Wordpress 4.6.1 zxcvbn.js is still version 1.0. The current version is 4.4.0.

#6 @swissspidy
14 months ago

  • Keywords needs-patch needs-testing added
  • Milestone changed from Awaiting Review to Future Release

#7 @pento
12 months ago

#39260 was marked as a duplicate.

#8 @pento
12 months ago

  • Milestone changed from Future Release to 4.8
  • Owner set to pento
  • Status changed from new to assigned
  • Version 4.6.1 deleted

@pento
12 months ago

#9 @pento
12 months ago

  • Keywords needs-patch needs-testing removed

31647.diff is the diff against zxcvbn master, to transform the word lists into a ROT13 version.

#10 @pento
12 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 39596:

Libraries: Update zxcvbn from version 1.0 to 4.4.1

This includes masses of bug fixes, as well as tweaks to how passwords are scored.

QUnit tests have been updated to reflect tha scoring changes.

Full changelog: https://github.com/dropbox/zxcvbn/compare/v1.0...v4.4.1

Fixes #31647.

#11 @jrchamp
9 months ago

@pento Is there any concern that the rot13 is negatively affecting the accuracy of the password strength evaluation? Specifically, won't this also affect the "keyboard layout" aka adjacency_graphs?

In case changes are needed, it might be worthwhile to also update the copyright line which was changed to "2012-2016 Dan Wheeler and Dropbox, Inc." in upstream.

Thank you for taking a look!

#12 @Otto42
7 months ago

@jrchamp ROT13 is reversible. The various lists are converted back to normal in memory before those lists are actually used, so this adjustment is basically a null change with zero impact. The only reason for doing the ROT13 is to hide the "naughty" parts of the password list from normal scanners and other tools that might see and/or block "bad words".

This ROT13 version otherwise operates identically to the original in every way.

#13 @jrchamp
7 months ago

@Otto42 Technically, the lists are not converted back to normal before use. They are only converted during use of the zxcvbn() function and stored in a temporary variable (meaning that the conversion must happen for each call to this function). The dictionary lists themselves are never modified, which is why matching.dictionary_match() is modified to ROT13 the password being checked so that the ROT13 substrings may be used against the dictionary lists directly (and the matching substring must then be re-ROT13'd to return the raw matching value). As long as no other code/functions directly access the dictionary lists, it should work fine, but this seems like a somewhat fragile assumption unless careful evaluation is done during each upgrade.

To answer my own question, the adjacency_graphs/spatial_match checks operate on the original user supplied password and do not appear to interact with the dictionaries directly. The main.coffee's misleading variable name "user_inputs" is not related to user input, but rather an optional dictionary list override.

Note: See TracTickets for help on using tickets.