From 24b20c10cebd25e61de8d4266c63fde94772e889 Mon Sep 17 00:00:00 2001 From: Muhsin Keloth Date: Wed, 30 Mar 2022 14:36:22 +0530 Subject: [PATCH] fix: Referer URL validation (#4309) Fixes #354 --- .rubocop.yml | 1 + app/models/campaign.rb | 11 +---------- app/models/conversation.rb | 8 ++++++++ lib/url_helper.rb | 11 +++++++++++ spec/helpers/url_helper_spec.rb | 15 +++++++++++++++ spec/models/conversation_spec.rb | 16 ++++++++++++++++ 6 files changed, 52 insertions(+), 10 deletions(-) create mode 100644 lib/url_helper.rb create mode 100644 spec/helpers/url_helper_spec.rb diff --git a/.rubocop.yml b/.rubocop.yml index c3096dd4a8ed..898f1ff24c5a 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -17,6 +17,7 @@ Metrics/ClassLength: - 'app/builders/messages/facebook/message_builder.rb' - 'app/controllers/api/v1/accounts/contacts_controller.rb' - 'app/listeners/action_cable_listener.rb' + - 'app/models/conversation.rb' RSpec/ExampleLength: Max: 25 Style/Documentation: diff --git a/app/models/campaign.rb b/app/models/campaign.rb index 304d270e757e..1201a79db398 100644 --- a/app/models/campaign.rb +++ b/app/models/campaign.rb @@ -33,8 +33,8 @@ # fk_rails_... (account_id => accounts.id) ON DELETE => cascade # fk_rails_... (inbox_id => inboxes.id) ON DELETE => cascade # -require 'uri' class Campaign < ApplicationRecord + include UrlHelper validates :account_id, presence: true validates :inbox_id, presence: true validates :title, presence: true @@ -94,15 +94,6 @@ def validate_url errors.add(:url, 'invalid') if inbox.inbox_type == 'Website' && !url_valid?(trigger_rules['url']) end - def url_valid?(url) - url = begin - URI.parse(url) - rescue StandardError - false - end - url.is_a?(URI::HTTP) || url.is_a?(URI::HTTPS) - end - def prevent_completed_campaign_from_update errors.add :status, 'The campaign is already completed' if !campaign_status_changed? && completed? end diff --git a/app/models/conversation.rb b/app/models/conversation.rb index 0296b563178e..75433f7fe227 100644 --- a/app/models/conversation.rb +++ b/app/models/conversation.rb @@ -46,12 +46,14 @@ class Conversation < ApplicationRecord include AssignmentHandler include RoundRobinHandler include ActivityMessageHandler + include UrlHelper validates :account_id, presence: true validates :inbox_id, presence: true before_validation :validate_additional_attributes validates :additional_attributes, jsonb_attributes_length: true validates :custom_attributes, jsonb_attributes_length: true + validate :validate_referer_url enum status: { open: 0, resolved: 1, pending: 2, snoozed: 3 } @@ -242,6 +244,12 @@ def mute_period 6.hours end + def validate_referer_url + return unless additional_attributes['referer'] + + self['additional_attributes']['referer'] = nil unless url_valid?(additional_attributes['referer']) + end + # creating db triggers trigger.before(:insert).for_each(:row) do "NEW.display_id := nextval('conv_dpid_seq_' || NEW.account_id);" diff --git a/lib/url_helper.rb b/lib/url_helper.rb new file mode 100644 index 000000000000..660ad9a04fa5 --- /dev/null +++ b/lib/url_helper.rb @@ -0,0 +1,11 @@ +require 'uri' +module UrlHelper + def url_valid?(url) + url = begin + URI.parse(url) + rescue StandardError + false + end + url.is_a?(URI::HTTP) || url.is_a?(URI::HTTPS) + end +end diff --git a/spec/helpers/url_helper_spec.rb b/spec/helpers/url_helper_spec.rb new file mode 100644 index 000000000000..14defbf17854 --- /dev/null +++ b/spec/helpers/url_helper_spec.rb @@ -0,0 +1,15 @@ +require 'rails_helper' + +describe UrlHelper, type: :helper do + describe '#url_valid' do + context 'when url valid called' do + it 'return if valid url passed' do + expect(helper.url_valid?('https://app.chatwoot.com/')).to eq true + end + + it 'return false if invalid url passed' do + expect(helper.url_valid?('javascript:alert(document.cookie)')).to eq false + end + end + end +end diff --git a/spec/models/conversation_spec.rb b/spec/models/conversation_spec.rb index 182073fbc3a9..6a900279563f 100644 --- a/spec/models/conversation_spec.rb +++ b/spec/models/conversation_spec.rb @@ -525,4 +525,20 @@ expect { notification.reload }.to raise_error ActiveRecord::RecordNotFound end end + + describe 'validate invalid referer url' do + let(:conversation) { create(:conversation, additional_attributes: { referer: 'javascript' }) } + + it 'returns nil' do + expect(conversation['additional_attributes']['referer']).to eq(nil) + end + end + + describe 'validate valid referer url' do + let(:conversation) { create(:conversation, additional_attributes: { referer: 'https://www.chatwoot.com/' }) } + + it 'returns nil' do + expect(conversation['additional_attributes']['referer']).to eq('https://www.chatwoot.com/') + end + end end