From: Tom Hughes <tom@compton.nu>
Date: Tue, 7 Nov 2023 17:16:21 +0000 (+0000)
Subject: Use secure_compare to compare passwords and tokens
X-Git-Tag: live~1093
X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/55a05d9e809775a6ea6305add096022ef1ec70e4?hp=-c

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.
---

55a05d9e809775a6ea6305add096022ef1ec70e4
diff --git a/lib/password_hash.rb b/lib/password_hash.rb
index de1f20d31..325955cf1 100644
--- 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 71fa4f2f1..087a117c3 100755
--- 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)