CVE-2022-1021: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting') (CWE-79)
value: Insecure Storage of Sensitive Information in GitHub repository chatwoot/chatwoot prior to 2.6.0.
CWE description: The product does not neutralize or incorrectly neutralizes user-controllable input before it is placed in output that is used as a web page that is served to other users.
CVSS v3 base score: 5.4
Published: 2022-08-19T13:15Z
Repository: https://github.com/chatwoot/chatwoot
Language: Ruby
Files changed: .rubocop.yml, app/models/campaign.rb, app/models/conversation.rb, lib/url_helper.rb, spec/helpers/url_helper_spec.rb, spec/models/conversation_spec.rb
Commit date: 2022-03-30 14:36:22 +0530
Commit message: fix: Referer URL validation (#4309)

Fixes #354
Diff: diff --git a/.rubocop.yml b/.rubocop.yml
index c3096dd4a..898f1ff24 100644
--- a/.rubocop.yml
+++ b/.rubocop.yml
@@ -15,10 +15,11 @@ Metrics/ClassLength:
     - 'app/mailers/conversation_reply_mailer.rb'
     - 'app/models/message.rb'
     - '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:
   Enabled: false
 Style/ExponentialNotation:
diff --git a/app/models/campaign.rb b/app/models/campaign.rb
index 304d270e7..1201a79db 100644
--- a/app/models/campaign.rb
+++ b/app/models/campaign.rb
@@ -31,12 +31,12 @@
 # Foreign Keys
 #
 #  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
   validates :message, presence: true
   validate :validate_campaign_inbox
@@ -92,19 +92,10 @@ class Campaign < ApplicationRecord
     return unless trigger_rules['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
 
   # creating db triggers
diff --git a/app/models/conversation.rb b/app/models/conversation.rb
index 0296b5631..75433f7fe 100644
--- a/app/models/conversation.rb
+++ b/app/models/conversation.rb
@@ -44,16 +44,18 @@
 class Conversation < ApplicationRecord
   include Labelable
   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 }
 
   scope :latest, -> { order(last_activity_at: :desc) }
   scope :unassigned, -> { where(assignee_id: nil) }
@@ -240,10 +242,16 @@ class Conversation < ApplicationRecord
 
   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);"
   end
 end
diff --git a/lib/url_helper.rb b/lib/url_helper.rb
new file mode 100644
index 000000000..660ad9a04
--- /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 000000000..14defbf17
--- /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 182073fbc..6a9002795 100644
--- a/spec/models/conversation_spec.rb
+++ b/spec/models/conversation_spec.rb
@@ -523,6 +523,22 @@ RSpec.describe Conversation, type: :model do
     it 'delete associated notifications if conversation is deleted' do
       conversation.destroy!
       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

Vulnerable code: require 'uri'
  def url_valid?(url)
    url = begin
      URI.parse(url)
    rescue StandardError
      false
    end
    url.is_a?(URI::HTTP) || url.is_a?(URI::HTTPS)
  end

Fixed code:     - 'app/models/conversation.rb'
  include UrlHelper
  include UrlHelper
  validate :validate_referer_url
  def validate_referer_url
    return unless additional_attributes['referer']

    self['additional_attributes']['referer'] = nil unless url_valid?(additional_attributes['referer'])
  end

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

  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