From 55a05d9e809775a6ea6305add096022ef1ec70e4 Mon Sep 17 00:00:00 2001 From: Tom Hughes Date: Tue, 7 Nov 2023 17:16:21 +0000 Subject: [PATCH] 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. --- lib/password_hash.rb | 6 +++--- script/deliver-message | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) 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) -- 2.39.5