Use secure_compare to compare passwords and tokens

It's unlikely there is an explotable attack here given than network
latencies and variability will swamp any local timing differences but
it's best practice and there's no reason not to.

diff --git a/lib/password_hash.rb b/lib/password_hash.rb
index de1f20d317afec7e1f2429e30af67ced716fdb2a..325955cf114f1a207bf93a9dbf8f58a11d16fed0 100644 (file)
--- a/lib/password_hash.rb
+++ b/lib/password_hash.rb
@@ -16,13 +16,13 @@ module PasswordHash
     if Argon2::HashFormat.valid_hash?(hash)
       Argon2::Password.verify_password(candidate, hash)
     elsif salt.nil?
-      hash == Digest::MD5.hexdigest(candidate)
+      ActiveSupport::SecurityUtils.secure_compare(hash, Digest::MD5.hexdigest(candidate))
     elsif salt.include?("!")
       algorithm, iterations, salt = salt.split("!")
       size = Base64.strict_decode64(hash).length
-      hash == pbkdf2(candidate, salt, iterations.to_i, size, algorithm)
+      ActiveSupport::SecurityUtils.secure_compare(hash, pbkdf2(candidate, salt, iterations.to_i, size, algorithm))
     else
-      hash == Digest::MD5.hexdigest(salt + candidate)
+      ActiveSupport::SecurityUtils.secure_compare(hash, Digest::MD5.hexdigest(salt + candidate))
     end
   end
 

diff --git a/script/deliver-message b/script/deliver-message
index 71fa4f2f15b1e500d5ca55ca2be19f5db0658987..087a117c3dceeeba884d600e8107de05aa77d950 100755 (executable)
--- a/script/deliver-message
+++ b/script/deliver-message
@@ -20,8 +20,8 @@ else
   exit 0
 end
 
+exit 0 unless ActiveSupport::SecurityUtils.secure_compare(token, digest[0, 6])
 exit 0 unless from.active?
-exit 0 unless token == digest[0, 6]
 exit 0 if date < 1.month.ago
 
 message&.update(:message_read => true)