diff --git a/app/controllers/public/api/v1/inboxes/contacts_controller.rb b/app/controllers/public/api/v1/inboxes/contacts_controller.rb index 835c2596b..28bb08fa6 100644 --- a/app/controllers/public/api/v1/inboxes/contacts_controller.rb +++ b/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 diff --git a/spec/controllers/public/api/v1/inbox/contacts_controller_spec.rb b/spec/controllers/public/api/v1/inbox/contacts_controller_spec.rb index 244d31c4d..e0fdfee81 100644 --- a/spec/controllers/public/api/v1/inbox/contacts_controller_spec.rb +++ b/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