fix(public-api): bind hmac verification to contact identifier

app/controllers/public/api/v1/inboxes/contacts_controller.rb

@@ -35,11 +35,25 @@ class Public::Api::V1::Inboxes::ContactsController < Public::Api::V1::InboxesCon
   end
 
   def valid_hmac?
-    params[:identifier_hash] == OpenSSL::HMAC.hexdigest(
+    expected_identifier = hmac_identifier
+    return false if expected_identifier.blank?
+    return false unless params[:identifier].to_s == expected_identifier
+
+    expected_hash = OpenSSL::HMAC.hexdigest(
       'sha256',
       @inbox_channel.hmac_token,
-      params[:identifier].to_s
+      expected_identifier
     )
+    identifier_hash = params[:identifier_hash].to_s
+    return false unless identifier_hash.bytesize == expected_hash.bytesize
+
+    ActiveSupport::SecurityUtils.secure_compare(identifier_hash, expected_hash)
+  end
+
+  def hmac_identifier
+    return params[:identifier].to_s if @contact_inbox.blank?
+
+    @contact_inbox.contact.identifier.to_s
   end
 
   def permitted_params

spec/controllers/public/api/v1/inbox/contacts_controller_spec.rb

@@ -2,7 +2,7 @@ require 'rails_helper'
 
 RSpec.describe 'Public Inbox Contacts API', type: :request do
   let!(:api_channel) { create(:channel_api) }
-  let!(:contact) { create(:contact) }
+  let!(:contact) { create(:contact, account: api_channel.account, identifier: 'victim-identifier') }
   let!(:contact_inbox) { create(:contact_inbox, contact: contact, inbox: api_channel.inbox) }
 
   describe 'POST /public/api/v1/inboxes/{identifier}/contact' do
@@ -36,6 +36,26 @@ RSpec.describe 'Public Inbox Contacts API', type: :request do
       expect(data['source_id']).to eq contact_inbox.source_id
       expect(data['pubsub_token']).to eq contact_inbox.pubsub_token
     end
+
+    it 'marks the contact inbox verified when the identifier hash matches the selected contact' do
+      identifier_hash = OpenSSL::HMAC.hexdigest('sha256', api_channel.hmac_token, contact.identifier)
+
+      get "/public/api/v1/inboxes/#{api_channel.identifier}/contacts/#{contact_inbox.source_id}",
+          params: { identifier: contact.identifier, identifier_hash: identifier_hash }
+
+      expect(response).to have_http_status(:success)
+      expect(contact_inbox.reload.hmac_verified).to be(true)
+    end
+
+    it 'does not verify a contact inbox with a hash for a different identifier' do
+      attacker_hash = OpenSSL::HMAC.hexdigest('sha256', api_channel.hmac_token, 'attacker-identifier')
+
+      get "/public/api/v1/inboxes/#{api_channel.identifier}/contacts/#{contact_inbox.source_id}",
+          params: { identifier: 'attacker-identifier', identifier_hash: attacker_hash }
+
+      expect(response).to have_http_status(:internal_server_error)
+      expect(contact_inbox.reload.hmac_verified).to be(false)
+    end
   end
 
   describe 'PATCH /public/api/v1/inboxes/{identifier}/contact/{source_id}' do