refactor implementation to avoid thread safety issues for user inputs

Gemfile.lock

@@ -12,6 +12,7 @@ GEM
     diff-lcs (1.4.4)
     docile (1.4.0)
     json (2.6.3)
+    libv8-node (16.10.0.0-arm64-darwin)
     libv8-node (16.10.0.0-x86_64-darwin-19)
     libv8-node (16.10.0.0-x86_64-linux)
     method_source (1.0.0)
@@ -65,6 +66,7 @@ GEM
     unicode-display_width (2.4.2)
 
 PLATFORMS
+  arm64-darwin-21
   x86_64-darwin-19
   x86_64-linux
 

README.md

@@ -95,7 +95,7 @@ To install this gem onto your local machine, run `bundle exec rake install`. To
 
 ## Contributing
 
-Bug reports and pull requests are welcome on GitHub at https://github.com/formigarafa/zxcvbn. This project is intended to be a safe, welcoming space for collaboration, and contributors are expected to adhere to the [code of conduct](https://github.com/[USERNAME]/zxcvbn/blob/master/CODE_OF_CONDUCT.md).
+Bug reports and pull requests are welcome on GitHub at https://github.com/formigarafa/zxcvbn-rb. This project is intended to be a safe, welcoming space for collaboration, and contributors are expected to adhere to the [code of conduct](https://github.com/[USERNAME]/zxcvbn/blob/master/CODE_OF_CONDUCT.md).
 
 ## License
 

lib/zxcvbn.rb

@@ -13,13 +13,7 @@ class Error < StandardError; end
 
   def self.zxcvbn(password, user_inputs = [])
     start = (Time.now.to_f * 1000).to_i
-    # reset the user inputs matcher on a per-request basis to keep things stateless
-    sanitized_inputs = []
-    user_inputs.each do |arg|
-      sanitized_inputs << arg.to_s.downcase if arg.is_a?(String) || arg.is_a?(Numeric) || arg == true || arg == false
-    end
-    Matching.user_input_dictionary = sanitized_inputs
-    matches = Matching.omnimatch(password)
+    matches = Matching.omnimatch(password, user_inputs)
     result = Scoring.most_guessable_match_sequence(password, matches)
     result["calc_time"] = (Time.now.to_f * 1000).to_i - start
     attack_times = TimeEstimates.estimate_attack_times(result["guesses"])

lib/zxcvbn/matching.rb

@@ -128,8 +128,15 @@ def self.sorted(matches)
     # ------------------------------------------------------------------------------
     # omnimatch -- combine everything ----------------------------------------------
     # ------------------------------------------------------------------------------
-    def self.omnimatch(password)
+    def self.omnimatch(password, user_inputs = [])
+      user_dict = build_user_input_dictionary(user_inputs)
       matches = []
+      user_input_matchers = [
+        :dictionary_match,
+        :reverse_dictionary_match,
+        :l33t_match,
+        :repeat_match
+      ]
       matchers = [
         :dictionary_match,
         :reverse_dictionary_match,
@@ -141,51 +148,60 @@ def self.omnimatch(password)
         :date_match
       ]
       matchers.each do |matcher|
-        matches += send(matcher, password)
+        matches += if user_input_matchers.include?(matcher)
+          send(matcher, password, user_dict)
+        else
+          send(matcher, password)
+        end
       end
       sorted(matches)
     end
 
     #-------------------------------------------------------------------------------
     # dictionary match (common passwords, english, last names, etc) ----------------
     #-------------------------------------------------------------------------------
-    def self.dictionary_match(password, _ranked_dictionaries = RANKED_DICTIONARIES)
+    def self.dictionary_match(password, user_dict, _ranked_dictionaries = RANKED_DICTIONARIES)
       # _ranked_dictionaries variable is for unit testing purposes
       matches = []
+      _ranked_dictionaries.each do |dictionary_name, ranked_dict|
+        check_dictionary(matches, password, dictionary_name, ranked_dict)
+      end
+      check_dictionary(matches, password, "user_inputs", user_dict)
+      sorted(matches)
+    end
+
+    def self.check_dictionary(matches, password, dictionary_name, ranked_dict)
       len = password.length
       password_lower = password.downcase
-      _ranked_dictionaries.each do |dictionary_name, ranked_dict|
-        longest_dict_word_size = RANKED_DICTIONARIES_MAX_WORD_SIZE.fetch(dictionary_name) do
-          ranked_dict.keys.max_by(&:size)&.size || 0
-        end
-        search_width = [longest_dict_word_size, len].min
-        (0...len).each do |i|
-          search_end = [i + search_width, len].min
-          (i...search_end).each do |j|
-            if ranked_dict.key?(password_lower[i..j])
-              word = password_lower[i..j]
-              rank = ranked_dict[word]
-              matches << {
-                "pattern" => "dictionary",
-                "i" => i,
-                "j" => j,
-                "token" => password[i..j],
-                "matched_word" => word,
-                "rank" => rank,
-                "dictionary_name" => dictionary_name,
-                "reversed" => false,
-                "l33t" => false
-              }
-            end
+      longest_word_size = RANKED_DICTIONARIES_MAX_WORD_SIZE.fetch(dictionary_name) do
+        ranked_dict.keys.max_by(&:size)&.size || 0
+      end
+      search_width = [longest_word_size, len].min
+      (0...len).each do |i|
+        search_end = [i + search_width, len].min
+        (i...search_end).each do |j|
+          if ranked_dict.key?(password_lower[i..j])
+            word = password_lower[i..j]
+            rank = ranked_dict[word]
+            matches << {
+              "pattern" => "dictionary",
+              "i" => i,
+              "j" => j,
+              "token" => password[i..j],
+              "matched_word" => word,
+              "rank" => rank,
+              "dictionary_name" => dictionary_name,
+              "reversed" => false,
+              "l33t" => false
+            }
           end
         end
       end
-      sorted(matches)
     end
 
-    def self.reverse_dictionary_match(password, _ranked_dictionaries = RANKED_DICTIONARIES)
+    def self.reverse_dictionary_match(password, user_dict, _ranked_dictionaries = RANKED_DICTIONARIES)
       reversed_password = password.reverse
-      matches = dictionary_match(reversed_password, _ranked_dictionaries)
+      matches = dictionary_match(reversed_password, user_dict, _ranked_dictionaries)
       matches.each do |match|
         match["token"] = match["token"].reverse
         match["reversed"] = true
@@ -195,10 +211,15 @@ def self.reverse_dictionary_match(password, _ranked_dictionaries = RANKED_DICTIO
       sorted(matches)
     end
 
-    def self.user_input_dictionary=(ordered_list)
-      ranked_dict = build_ranked_dict(ordered_list.dup)
-      RANKED_DICTIONARIES["user_inputs"] = ranked_dict
-      RANKED_DICTIONARIES_MAX_WORD_SIZE["user_inputs"] = ranked_dict.keys.max_by(&:size)&.size || 0
+    def self.build_user_input_dictionary(user_inputs_or_dict)
+      # optimization: if we receive a hash, we've been given the dict back (from the repeat matcher)
+      return user_inputs_or_dict if user_inputs_or_dict.is_a?(Hash)
+
+      sanitized_inputs = []
+      user_inputs_or_dict.each do |arg|
+        sanitized_inputs << arg.to_s.downcase if arg.is_a?(String) || arg.is_a?(Numeric) || arg == true || arg == false
+      end
+      build_ranked_dict(sanitized_inputs)
     end
 
     #-------------------------------------------------------------------------------
@@ -287,13 +308,13 @@ def self.enumerate_l33t_subs(table)
       sub_dicts
     end
 
-    def self.l33t_match(password, _ranked_dictionaries = RANKED_DICTIONARIES, _l33t_table = L33T_TABLE)
+    def self.l33t_match(password, user_dict, _ranked_dictionaries = RANKED_DICTIONARIES, _l33t_table = L33T_TABLE)
       matches = []
       enumerate_l33t_subs(relevant_l33t_subtable(password, _l33t_table)).each do |sub|
         break if sub.empty? # corner case: password has no relevant subs.
 
         subbed_password = translate(password, sub)
-        dictionary_match(subbed_password, _ranked_dictionaries).each do |match|
+        dictionary_match(subbed_password, user_dict, _ranked_dictionaries).each do |match|
           token = password[match["i"]..match["j"]]
           if token.downcase == match["matched_word"]
             next # only return the matches that contain an actual substitution
@@ -403,7 +424,7 @@ def self.spatial_match_helper(password, graph, graph_name)
     #-------------------------------------------------------------------------------
     # repeats (aaa, abcabcabc) and sequences (abcdef) ------------------------------
     #-------------------------------------------------------------------------------
-    def self.repeat_match(password)
+    def self.repeat_match(password, user_dict)
       matches = []
       greedy = /(.+)\1+/
       lazy = /(.+?)\1+/
@@ -436,7 +457,7 @@ def self.repeat_match(password)
         i = match.begin(0)
         j = match.end(0) - 1
         # recursively match and score the base string
-        base_analysis = Scoring.most_guessable_match_sequence(base_token, omnimatch(base_token))
+        base_analysis = Scoring.most_guessable_match_sequence(base_token, omnimatch(base_token, user_dict))
         base_matches = base_analysis["sequence"]
         base_guesses = base_analysis["guesses"]
         matches << {

spec/matching_spec.rb

@@ -122,7 +122,7 @@ def check_matches(prefix, rspec_it, matches, pattern_names, patterns, ijs, props
     }
   }
 
-  dm = ->(pw) { Zxcvbn::Matching.dictionary_match(pw, test_dicts) }
+  dm = ->(pw) { Zxcvbn::Matching.dictionary_match(pw, {}, test_dicts) }
 
   matches = dm.call("motherboard")
   patterns = ["mother", "motherboard", "board"]
@@ -218,7 +218,7 @@ def check_matches(prefix, rspec_it, matches, pattern_names, patterns, ijs, props
   end
 
   # test the default dictionaries
-  matches = Zxcvbn::Matching.dictionary_match("wow")
+  matches = Zxcvbn::Matching.dictionary_match("wow", {})
   patterns = ["wow"]
   ijs = [[0, 2]]
   msg = "default dictionaries"
@@ -236,9 +236,8 @@ def check_matches(prefix, rspec_it, matches, pattern_names, patterns, ijs, props
     }
   )
 
-  Zxcvbn::Matching.user_input_dictionary = ["foo", "bar"]
-  matches = Zxcvbn::Matching.dictionary_match "foobar"
-  Zxcvbn::Matching.user_input_dictionary = []
+  user_dict = Zxcvbn::Matching.build_user_input_dictionary(["foo", "bar"])
+  matches = Zxcvbn::Matching.dictionary_match("foobar", user_dict)
   matches = matches.select { |match| match["dictionary_name"] == "user_inputs" }
   msg = "matches with provided user input dictionary"
   check_matches(
@@ -265,7 +264,7 @@ def check_matches(prefix, rspec_it, matches, pattern_names, patterns, ijs, props
     }
   }
   password = "0123456789"
-  matches = Zxcvbn::Matching.reverse_dictionary_match password, test_dicts
+  matches = Zxcvbn::Matching.reverse_dictionary_match(password, {}, test_dicts)
   msg = "matches against reversed words"
   check_matches(
     msg,
@@ -327,7 +326,7 @@ def check_matches(prefix, rspec_it, matches, pattern_names, patterns, ijs, props
     }
   }
 
-  lm = ->(pw) { Zxcvbn::Matching.l33t_match(pw, dicts, test_table) }
+  lm = ->(pw) { Zxcvbn::Matching.l33t_match(pw, {}, dicts, test_table) }
 
   it "doesn't match ''" do
     expect(lm.call("")).to eq([])
@@ -383,7 +382,7 @@ def check_matches(prefix, rspec_it, matches, pattern_names, patterns, ijs, props
   end
 
   it "doesn't match single-character l33ted words" do
-    matches = Zxcvbn::Matching.l33t_match("4 1 @")
+    matches = Zxcvbn::Matching.l33t_match("4 1 @", {})
     expect(matches).to eq([])
   end
 
@@ -534,7 +533,7 @@ def check_matches(prefix, rspec_it, matches, pattern_names, patterns, ijs, props
 RSpec.describe "repeat matching" do
   ["", "#"].each do |password|
     it "doesn't match length-#{password.length} repeat patterns" do
-      expect(Zxcvbn::Matching.repeat_match(password)).to eq([])
+      expect(Zxcvbn::Matching.repeat_match(password, {})).to eq([])
     end
   end
 
@@ -543,7 +542,7 @@ def check_matches(prefix, rspec_it, matches, pattern_names, patterns, ijs, props
   suffixes = ["u", "u%7"]
   pattern = "&&&&&"
   genpws(pattern, prefixes, suffixes).each do |password, i, j|
-    matches = Zxcvbn::Matching.repeat_match password
+    matches = Zxcvbn::Matching.repeat_match(password, {})
     msg = "matches embedded repeat patterns"
     check_matches(
       msg,
@@ -559,7 +558,7 @@ def check_matches(prefix, rspec_it, matches, pattern_names, patterns, ijs, props
   [3, 12].each do |length|
     ["a", "Z", "4", "&"].each do |chr|
       pattern = chr * length
-      matches = Zxcvbn::Matching.repeat_match pattern
+      matches = Zxcvbn::Matching.repeat_match(pattern, {})
       msg = "matches repeats with base character '#{chr}'"
       check_matches(
         msg,
@@ -573,7 +572,7 @@ def check_matches(prefix, rspec_it, matches, pattern_names, patterns, ijs, props
     end
   end
 
-  matches = Zxcvbn::Matching.repeat_match "BBB1111aaaaa@@@@@@"
+  matches = Zxcvbn::Matching.repeat_match("BBB1111aaaaa@@@@@@", {})
   patterns = ["BBB", "1111", "aaaaa", "@@@@@@"]
   msg = "matches multiple adjacent repeats"
   check_matches(
@@ -586,7 +585,7 @@ def check_matches(prefix, rspec_it, matches, pattern_names, patterns, ijs, props
     { "base_token" => ["B", "1", "a", "@"] }
   )
 
-  matches = Zxcvbn::Matching.repeat_match "2818BBBbzsdf1111@*&@!aaaaaEUDA@@@@@@1729"
+  matches = Zxcvbn::Matching.repeat_match("2818BBBbzsdf1111@*&@!aaaaaEUDA@@@@@@1729", {})
   msg = "matches multiple repeats with non-repeats in-between"
   check_matches(
     msg,
@@ -600,7 +599,7 @@ def check_matches(prefix, rspec_it, matches, pattern_names, patterns, ijs, props
 
   # test multi-character repeats
   pattern = "abab"
-  matches = Zxcvbn::Matching.repeat_match pattern
+  matches = Zxcvbn::Matching.repeat_match(pattern, {})
   msg = "matches multi-character repeat pattern"
   check_matches(
     msg,
@@ -613,7 +612,7 @@ def check_matches(prefix, rspec_it, matches, pattern_names, patterns, ijs, props
   )
 
   pattern = "aabaab"
-  matches = Zxcvbn::Matching.repeat_match pattern
+  matches = Zxcvbn::Matching.repeat_match(pattern, {})
   msg = "matches aabaab as a repeat instead of the aa prefix"
   check_matches(
     msg,
@@ -626,7 +625,7 @@ def check_matches(prefix, rspec_it, matches, pattern_names, patterns, ijs, props
   )
 
   pattern = "abababab"
-  matches = Zxcvbn::Matching.repeat_match pattern
+  matches = Zxcvbn::Matching.repeat_match(pattern, {})
   msg = "identifies ab as repeat string, even though abab is also repeated"
   check_matches(
     msg,
@@ -834,11 +833,11 @@ def check_matches(prefix, rspec_it, matches, pattern_names, patterns, ijs, props
 
 RSpec.describe "omnimatch" do
   it "doesn't match ''" do
-    expect(Zxcvbn::Matching.omnimatch("")).to eq([])
+    expect(Zxcvbn::Matching.omnimatch("", [])).to eq([])
   end
 
   password = "r0sebudmaelstrom11/20/91aaaa"
-  matches = Zxcvbn::Matching.omnimatch(password)
+  matches = Zxcvbn::Matching.omnimatch(password, [])
   [
     ["dictionary",  [0, 6]],
     ["dictionary",  [7, 15]],

spec/scoring_spec.rb

@@ -215,7 +215,7 @@
   ].each do |token, base_token, repeat_count|
     base_guesses = Zxcvbn::Scoring.most_guessable_match_sequence(
       base_token,
-      Zxcvbn::Matching.omnimatch(base_token)
+      Zxcvbn::Matching.omnimatch(base_token, {})
     )["guesses"]
     match = {
       "token" => token,