Merge pull request #1075 from texpert/sanitize-attrs

Sanitize name and description attrs of TermTaxonomy classes to prevent XSS attacks

.rubocop.yml

@@ -29,6 +29,9 @@ Metrics/MethodLength:
   Exclude:
     - spec/**/*
 
+RSpec/AnyInstance:
+  Enabled: false
+
 RSpec/ExampleLength:
   Enabled: false
 

CHANGELOG.md

@@ -8,6 +8,10 @@
 - On cama_site_check_existence, if site is unknown, use `allow_other_host: true` for redirection to main site
   - Starting from Rails 7.0 a redirection to other host will raise an exception unless the `redirect_to` method is 
     called with the `allow_other_host: true` option
+- Set sprocket-rails version to be at least 3.5.1
+- Use MiniMime for mime types, because the MiniMagick 5.0 has no Image#mime_type
+- Reimplement the temporary uploaded file removing, wrapping it in a bl…ock to make possible overriding the block in the app initializer to use an async job
+- Sanitize name and description attrs of TermTaxonomy classes to prevent XSS attacks
 
 ## [2.7.5](https://github.com/owen2345/camaleon-cms/tree/2.7.5) (2023-11-22)
 - Fix the test email for non-main sites by [brian-kephart](https://github.com/brian-kephart) in [\#1050](https://github.com/owen2345/camaleon-cms/pull/1050)

app/models/camaleon_cms/term_taxonomy.rb

@@ -16,6 +16,19 @@ def self.inherited(subclass)
     # attr_accessible :data_options
     # attr_accessible :data_metas
 
+    # TODO: Remove the 1st branch when support will be dropped of Rails < 7.1
+    if ::Rails::VERSION::STRING < '7.1.0'
+      before_validation(on: %i[create update]) do
+        %i[name description].each do |attr|
+          next unless new_record? || attribute_changed?(attr)
+
+          self[attr] = ActionController::Base.helpers.sanitize(__send__(attr))
+        end
+      end
+    else
+      normalizes :name, :description, with: ->(field) { ActionController::Base.helpers.sanitize(field) }
+    end
+
     # callbacks
     before_validation :before_validating
     before_destroy :destroy_dependencies

spec/models/category_spec.rb

@@ -0,0 +1,10 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+require 'shared_specs/sanitize_attrs'
+
+RSpec.describe CamaleonCms::Category, type: :model do
+  before { allow_any_instance_of(described_class).to receive(:set_site) }
+
+  it_behaves_like 'sanitize attrs', model: described_class, attrs_to_sanitize: %i[name description]
+end

spec/models/nav_menu_item_spec.rb

@@ -0,0 +1,10 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+require 'shared_specs/sanitize_attrs'
+
+RSpec.describe CamaleonCms::NavMenuItem, type: :model do
+  before { allow_any_instance_of(described_class).to receive(:update_count) }
+
+  it_behaves_like 'sanitize attrs', model: described_class, attrs_to_sanitize: %i[name description]
+end

spec/models/nav_menu_spec.rb

@@ -0,0 +1,8 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+require 'shared_specs/sanitize_attrs'
+
+RSpec.describe CamaleonCms::NavMenu, type: :model do
+  it_behaves_like 'sanitize attrs', model: described_class, attrs_to_sanitize: %i[name description]
+end

spec/models/plugin_spec.rb

@@ -0,0 +1,8 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+require 'shared_specs/sanitize_attrs'
+
+RSpec.describe CamaleonCms::Plugin, type: :model do
+  it_behaves_like 'sanitize attrs', model: described_class, attrs_to_sanitize: %i[name description]
+end

spec/models/post_tag_spec.rb

@@ -0,0 +1,8 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+require 'shared_specs/sanitize_attrs'
+
+RSpec.describe CamaleonCms::PostTag, type: :model do
+  it_behaves_like 'sanitize attrs', model: described_class, attrs_to_sanitize: %i[name description]
+end

spec/models/post_type_spec.rb

@@ -0,0 +1,10 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+require 'shared_specs/sanitize_attrs'
+
+RSpec.describe CamaleonCms::PostType, type: :model do
+  init_site
+
+  it_behaves_like 'sanitize attrs', model: described_class, attrs_to_sanitize: %i[name description]
+end

spec/models/site_spec.rb

@@ -1,8 +1,11 @@
 # frozen_string_literal: true
 
 require 'rails_helper'
+require 'shared_specs/sanitize_attrs'
 
 RSpec.describe CamaleonCms::Site, type: :model do
+  it_behaves_like 'sanitize attrs', model: described_class, attrs_to_sanitize: %i[name description]
+
   describe 'check metas relationships' do
     let!(:site) { create(:site).decorate }
 

spec/models/term_taxonomy_spec.rb

@@ -0,0 +1,8 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+require 'shared_specs/sanitize_attrs'
+
+RSpec.describe CamaleonCms::TermTaxonomy, type: :model do
+  it_behaves_like 'sanitize attrs', model: described_class, attrs_to_sanitize: %i[name description]
+end

spec/models/theme_spec.rb

@@ -0,0 +1,8 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+require 'shared_specs/sanitize_attrs'
+
+RSpec.describe CamaleonCms::Theme, type: :model do
+  it_behaves_like 'sanitize attrs', model: described_class, attrs_to_sanitize: %i[name description]
+end

spec/models/user_role_spec.rb

@@ -0,0 +1,8 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+require 'shared_specs/sanitize_attrs'
+
+RSpec.describe CamaleonCms::UserRole, type: :model do
+  it_behaves_like 'sanitize attrs', model: described_class, attrs_to_sanitize: %i[name description]
+end

spec/models/widget/widget_main_spec.rb

@@ -0,0 +1,8 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+require 'shared_specs/sanitize_attrs'
+
+RSpec.describe CamaleonCms::Widget::Main, type: :model do
+  it_behaves_like 'sanitize attrs', model: described_class, attrs_to_sanitize: %i[name description]
+end

spec/models/widget/widget_sidebar_spec.rb

@@ -0,0 +1,8 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+require 'shared_specs/sanitize_attrs'
+
+RSpec.describe CamaleonCms::Widget::Sidebar, type: :model do
+  it_behaves_like 'sanitize attrs', model: described_class, attrs_to_sanitize: %i[name description]
+end

spec/shared_specs/sanitize_attrs.rb

@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+
+RSpec.shared_examples 'sanitize attrs' do |model:, attrs_to_sanitize:|
+  attrs_to_sanitize.each do |attr|
+    it 'sanitizes name attribute on create' do
+      attrs_for_creation = { attr => '"><script>alert(1)</script>' }
+      attrs_for_creation.merge!(site: @site) if defined?(@site)
+      model_instance = model.create(attrs_for_creation)
+
+      expect(model_instance.__send__(attr)).to eql('"&gt;alert(1)')
+    end
+
+    it 'sanitizes name attribute on update' do
+      attrs_for_creation = { attr => 'Legit text' }
+      attrs_for_creation.merge!(site: @site) if defined?(@site)
+      model_instance = model.create(attrs_for_creation)
+      # attrs_for_creation = { attr => '"><script>alert(1)</script>' }
+      model_instance.update(attr => '"><script>alert(1)</script>')
+
+      expect(model_instance.__send__(attr)).to eql('"&gt;alert(1)')
+    end
+  end
+end