CVE-2024-21636: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting') (CWE-79)
value: view_component is a framework for building reusable, testable, and encapsulated view components in Ruby on Rails. Versions prior to 3.9.0 and 2.83.0 have a cross-site scripting vulnerability that has the potential to impact anyone rendering a component directly from a controller with the view_component gem. Note that only components that define a `#call` method (i.e. instead of using a sidecar template) are affected. The return value of the `#call` method is not sanitized and can include user-defined content. In addition, the return value of the `#output_postamble` methodis not sanitized, which can also lead to cross-site scripting issues. Versions 3.9.0 and 2.83.0 have been released and fully mitigate both the `#call` and the `#output_postamble` vulnerabilities. As a workaround, sanitize the return value of `#call`.
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: 6.1
Published: 2024-01-04T20:15Z
Repository: https://github.com/ViewComponent/view_component
Language: Ruby
Files changed: .github/workflows/ci.yml, .rubocop.yml, docs/CHANGELOG.md, lib/view_component/base.rb, lib/view_component/compiler.rb, lib/view_component/test_helpers.rb, test/sandbox/app/components/after_render_component.rb, test/sandbox/app/components/content_predicate_component.rb, test/sandbox/app/components/custom_test_controller_component.rb, test/sandbox/app/components/inherited_from_uncompilable_component.rb, test/sandbox/app/components/inline_render_component.rb, test/sandbox/app/components/message_component.rb, test/sandbox/app/components/render_check_component.rb, test/sandbox/app/components/unsafe_component.rb, test/sandbox/app/components/unsafe_postamble_component.rb, test/sandbox/app/components/variant_ivar_component.rb, test/sandbox/app/controllers/integration_examples_controller.rb, test/sandbox/config/routes.rb, test/sandbox/test/collection_test.rb, test/sandbox/test/integration_test.rb, test/sandbox/test/rendering_test.rb, test/test_helper.rb
Commit date: 2024-01-04 11:25:54 -0800
Commit message: Ensure HTML output safety (#1950)


Diff: diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index bdb7dbbb..0434336b 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -87,11 +87,11 @@ jobs:
         CAPTURE_PATCH_ENABLED: ${{ matrix.mode == 'capture_patch_enabled' && 'true' || 'false' }}
     - name: Upload coverage results
       uses: actions/upload-artifact@v3.1.3
       if: always()
       with:
-        name: simplecov-resultset-rails${{matrix.rails_version}}-ruby${{matrix.ruby_version}}
+        name: simplecov-resultset-rails${{matrix.rails_version}}-ruby${{matrix.ruby_version}}-${{matrix.mode}}
         path: coverage
   primer_view_components_compatibility:
     name: Test compatibility with Primer ViewComponents (main)
     runs-on: ubuntu-latest
     steps:
diff --git a/.rubocop.yml b/.rubocop.yml
index 7f263864..93e28a2d 100644
--- a/.rubocop.yml
+++ b/.rubocop.yml
@@ -1,5 +1,6 @@
+ruby_version: 2.5
 require:
   - standard
   - "rubocop-md"
 inherit_gem:
   standard: config/base.yml
diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md
index 64acc5ca..477f3825 100644
--- a/docs/CHANGELOG.md
+++ b/docs/CHANGELOG.md
@@ -36,10 +36,14 @@ nav_order: 5
 
 * Avoid duplicating rendered string when `output_postamble` is blank.
 
     *Mitchell Henke*
 
+* Ensure HTML output safety.
+
+    *Cameron Dutro*
+
 ## 3.8.0
 
 * Use correct value for the `config.action_dispatch.show_exceptions` config option for edge Rails.
 
     *Cameron Dutro*
diff --git a/lib/view_component/base.rb b/lib/view_component/base.rb
index 8f7d2fe5..1dfc641f 100644
--- a/lib/view_component/base.rb
+++ b/lib/view_component/base.rb
@@ -104,13 +104,13 @@ module ViewComponent
       before_render
 
       if render?
         # Avoid allocating new string when output_postamble is blank
         if output_postamble.blank?
-          render_template_for(@__vc_variant).to_s
+          safe_render_template_for(@__vc_variant).to_s
         else
-          render_template_for(@__vc_variant).to_s + output_postamble
+          safe_render_template_for(@__vc_variant).to_s + safe_output_postamble
         end
       else
         ""
       end
     ensure
@@ -158,11 +158,11 @@ module ViewComponent
 
     # Optional content to be returned after the rendered template.
     #
     # @return [String]
     def output_postamble
-      ""
+      @@default_output_postamble ||= "".html_safe
     end
 
     # Called before rendering the component. Override to perform operations that
     # depend on having access to the view context, such as helpers.
     #
@@ -305,10 +305,42 @@ module ViewComponent
 
     def content_evaluated?
       defined?(@__vc_content_evaluated) && @__vc_content_evaluated
     end
 
+    def maybe_escape_html(text)
+      return text if request && !request.format.html?
+      return text if text.nil? || text.empty?
+
+      if text.html_safe?
+        text
+      else
+        yield
+        html_escape(text)
+      end
+    end
+
+    def safe_render_template_for(variant)
+      if compiler.renders_template_for_variant?(variant)
+        render_template_for(variant)
+      else
+        maybe_escape_html(render_template_for(variant)) do
+          Kernel.warn("WARNING: The #{self.class} component rendered HTML-unsafe output. The output will be automatically escaped, but you may want to investigate.")
+        end
+      end
+    end
+
+    def safe_output_postamble
+      maybe_escape_html(output_postamble) do
+        Kernel.warn("WARNING: The #{self.class} component was provided an HTML-unsafe postamble. The postamble will be automatically escaped, but you may want to investigate.")
+      end
+    end
+
+    def compiler
+      @compiler ||= self.class.compiler
+    end
+
     # Set the controller used for testing components:
     #
     # ```ruby
     # config.view_component.test_controller = "MyTestController"
     # ```
diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb
index 7295f532..d0847a0b 100644
--- a/lib/view_component/compiler.rb
+++ b/lib/view_component/compiler.rb
@@ -14,10 +14,11 @@ module ViewComponent
     class_attribute :mode, default: PRODUCTION_MODE
 
     def initialize(component_class)
       @component_class = component_class
       @redefinition_lock = Mutex.new
+      @variants_rendering_templates = Set.new
     end
 
     def compiled?
       CompileCache.compiled?(component_class)
     end
@@ -66,10 +67,11 @@ module ViewComponent
           RUBY
         end
       else
         templates.each do |template|
           method_name = call_method_name(template[:variant])
+          @variants_rendering_templates << template[:variant]
 
           redefinition_lock.synchronize do
             component_class.silence_redefinition_of_method(method_name)
             # rubocop:disable Style/EvalWithLocation
             component_class.class_eval <<-RUBY, template[:path], 0
@@ -87,10 +89,14 @@ module ViewComponent
       component_class.build_i18n_backend
 
       CompileCache.register(component_class)
     end
 
+    def renders_template_for_variant?(variant)
+      @variants_rendering_templates.include?(variant)
+    end
+
     private
 
     attr_reader :component_class, :redefinition_lock
 
     def define_render_template_for
diff --git a/lib/view_component/test_helpers.rb b/lib/view_component/test_helpers.rb
index 69d4b811..ae182629 100644
--- a/lib/view_component/test_helpers.rb
+++ b/lib/view_component/test_helpers.rb
@@ -176,17 +176,18 @@ module ViewComponent
     # ```
     #
     # @param full_path [String] The path to set for the current request.
     # @param host [String] The host to set for the current request.
     # @param method [String] The request method to set for the current request.
-    def with_request_url(full_path, host: nil, method: nil)
+    def with_request_url(full_path, host: nil, method: nil, format: :html)
       old_request_host = vc_test_request.host
       old_request_method = vc_test_request.request_method
       old_request_path_info = vc_test_request.path_info
       old_request_path_parameters = vc_test_request.path_parameters
       old_request_query_parameters = vc_test_request.query_parameters
       old_request_query_string = vc_test_request.query_string
+      old_request_format = vc_test_request.format.symbol
       old_controller = defined?(@vc_test_controller) && @vc_test_controller
 
       path, query = full_path.split("?", 2)
       vc_test_request.instance_variable_set(:@fullpath, full_path)
       vc_test_request.instance_variable_set(:@original_fullpath, full_path)
@@ -195,18 +196,20 @@ module ViewComponent
       vc_test_request.path_info = path
       vc_test_request.path_parameters = Rails.application.routes.recognize_path_with_request(vc_test_request, path, {})
       vc_test_request.set_header("action_dispatch.request.query_parameters",
         Rack::Utils.parse_nested_query(query).with_indifferent_access)
       vc_test_request.set_header(Rack::QUERY_STRING, query)
+      vc_test_request.format = format
       yield
     ensure
       vc_test_request.host = old_request_host
       vc_test_request.request_method = old_request_method
       vc_test_request.path_info = old_request_path_info
       vc_test_request.path_parameters = old_request_path_parameters
       vc_test_request.set_header("action_dispatch.request.query_parameters", old_request_query_parameters)
       vc_test_request.set_header(Rack::QUERY_STRING, old_request_query_string)
+      vc_test_request.format = old_request_format
       @vc_test_controller = old_controller
     end
 
     # Access the controller used by `render_inline`:
     #
diff --git a/test/sandbox/app/components/after_render_component.rb b/test/sandbox/app/components/after_render_component.rb
index 86f57d63..86117cb0 100644
--- a/test/sandbox/app/components/after_render_component.rb
+++ b/test/sandbox/app/components/after_render_component.rb
@@ -1,11 +1,11 @@
 # frozen_string_literal: true
 
 class AfterRenderComponent < ViewComponent::Base
   def call
-    "Hello, "
+    "Hello, ".html_safe
   end
 
   def output_postamble
-    "World!"
+    "World!".html_safe
   end
 end
diff --git a/test/sandbox/app/components/content_predicate_component.rb b/test/sandbox/app/components/content_predicate_component.rb
index b928ccaa..a2cb20be 100644
--- a/test/sandbox/app/components/content_predicate_component.rb
+++ b/test/sandbox/app/components/content_predicate_component.rb
@@ -3,9 +3,9 @@
 class ContentPredicateComponent < ViewComponent::Base
   def call
     if content?
       content
     else
-      "Default"
+      "Default".html_safe
     end
   end
 end
diff --git a/test/sandbox/app/components/custom_test_controller_component.rb b/test/sandbox/app/components/custom_test_controller_component.rb
index 9df474e3..1e66a559 100644
--- a/test/sandbox/app/components/custom_test_controller_component.rb
+++ b/test/sandbox/app/components/custom_test_controller_component.rb
@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class CustomTestControllerComponent < ViewComponent::Base
   def call
-    helpers.foo
+    html_escape(helpers.foo)
   end
 end
diff --git a/test/sandbox/app/components/inherited_from_uncompilable_component.rb b/test/sandbox/app/components/inherited_from_uncompilable_component.rb
index 1cf97d1f..271d3445 100644
--- a/test/sandbox/app/components/inherited_from_uncompilable_component.rb
+++ b/test/sandbox/app/components/inherited_from_uncompilable_component.rb
@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class InheritedFromUncompilableComponent < UncompilableComponent
   def call
-    "<div>hello world</div>"
+    "<div>hello world</div>".html_safe
   end
 end
diff --git a/test/sandbox/app/components/inline_render_component.rb b/test/sandbox/app/components/inline_render_component.rb
index 4b6934e4..bf25e364 100644
--- a/test/sandbox/app/components/inline_render_component.rb
+++ b/test/sandbox/app/components/inline_render_component.rb
@@ -4,8 +4,8 @@ class InlineRenderComponent < ViewComponent::Base
   def initialize(items:)
     @items = items
   end
 
   def call
-    @items.map { |c| render(c) }.join
+    @items.map { |c| render(c) }.join.html_safe
   end
 end
diff --git a/test/sandbox/app/components/message_component.rb b/test/sandbox/app/components/message_component.rb
index 93c0cdb4..28ad85d0 100644
--- a/test/sandbox/app/components/message_component.rb
+++ b/test/sandbox/app/components/message_component.rb
@@ -4,8 +4,8 @@ class MessageComponent < ViewComponent::Base
   def initialize(message:)
     @message = message
   end
 
   def call
-    @message
+    html_escape(@message)
   end
 end
diff --git a/test/sandbox/app/components/render_check_component.rb b/test/sandbox/app/components/render_check_component.rb
index 80b26e27..98de22bc 100644
--- a/test/sandbox/app/components/render_check_component.rb
+++ b/test/sandbox/app/components/render_check_component.rb
@@ -4,8 +4,8 @@ class RenderCheckComponent < ViewComponent::Base
   def render?
     !view_context.cookies[:hide]
   end
 
   def call
-    "Rendered"
+    "Rendered".html_safe
   end
 end
diff --git a/test/sandbox/app/components/unsafe_component.rb b/test/sandbox/app/components/unsafe_component.rb
new file mode 100644
index 00000000..df09ff1d
--- /dev/null
+++ b/test/sandbox/app/components/unsafe_component.rb
@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+class UnsafeComponent < ViewComponent::Base
+  def call
+    user_input = "<script>alert('hello!')</script>"
+
+    "<div>hello #{user_input}</div>"
+  end
+end
diff --git a/test/sandbox/app/components/unsafe_postamble_component.rb b/test/sandbox/app/components/unsafe_postamble_component.rb
new file mode 100644
index 00000000..522b1489
--- /dev/null
+++ b/test/sandbox/app/components/unsafe_postamble_component.rb
@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+class UnsafePostambleComponent < ViewComponent::Base
+  def call
+    "<div>some content</div>".html_safe
+  end
+
+  def output_postamble
+    "<script>alert('hello!')</script>"
+  end
+end
diff --git a/test/sandbox/app/components/variant_ivar_component.rb b/test/sandbox/app/components/variant_ivar_component.rb
index 386ca917..4d1cac5f 100644
--- a/test/sandbox/app/components/variant_ivar_component.rb
+++ b/test/sandbox/app/components/variant_ivar_component.rb
@@ -4,8 +4,8 @@ class VariantIvarComponent < ViewComponent::Base
   def initialize(variant:)
     @variant = variant
   end
 
   def call
-    @variant.to_s
+    html_escape(@variant.to_s)
   end
 end
diff --git a/test/sandbox/app/controllers/integration_examples_controller.rb b/test/sandbox/app/controllers/integration_examples_controller.rb
index 6489f811..ae654faf 100644
--- a/test/sandbox/app/controllers/integration_examples_controller.rb
+++ b/test/sandbox/app/controllers/integration_examples_controller.rb
@@ -57,6 +57,14 @@ class IntegrationExamplesController < ActionController::Base
   end
 
   def inherited_from_uncompilable_component
     render(InheritedFromUncompilableComponent.new)
   end
+
+  def unsafe_component
+    render(UnsafeComponent.new)
+  end
+
+  def unsafe_postamble_component
+    render(UnsafePostambleComponent.new)
+  end
 end
diff --git a/test/sandbox/config/routes.rb b/test/sandbox/config/routes.rb
index afbb357b..f671f7ad 100644
--- a/test/sandbox/config/routes.rb
+++ b/test/sandbox/config/routes.rb
@@ -27,10 +27,12 @@ Sandbox::Application.routes.draw do
   get :link_to_helper, to: "integration_examples#link_to_helper"
   get :cached_capture, to: "integration_examples#cached_capture"
   get :cached_partial, to: "integration_examples#cached_partial"
   get :inherited_sidecar, to: "integration_examples#inherited_sidecar"
   get :inherited_from_uncompilable_component, to: "integration_examples#inherited_from_uncompilable_component"
+  get :unsafe_component, to: "integration_examples#unsafe_component"
+  get :unsafe_postamble_component, to: "integration_examples#unsafe_postamble_component"
   post :create, to: "integration_examples#create"
 
   constraints(lambda { |request| request.env["warden"].authenticate! }) do
     get :constraints_with_env, to: "integration_examples#index"
   end
diff --git a/test/sandbox/test/collection_test.rb b/test/sandbox/test/collection_test.rb
index 42742b2d..bad2b0aa 100644
--- a/test/sandbox/test/collection_test.rb
+++ b/test/sandbox/test/collection_test.rb
@@ -10,11 +10,11 @@ module ViewComponent
       def initialize(**attributes)
         self.product = attributes[:product]
       end
 
       def call
-        "<div data-name='#{product.name}'><h1>#{product.name}</h1></div>"
+        "<div data-name='#{product.name}'><h1>#{product.name}</h1></div>".html_safe
       end
     end
 
     def setup
       @products = [OpenStruct.new(name: "Radio clock"), OpenStruct.new(name: "Mints")]
diff --git a/test/sandbox/test/integration_test.rb b/test/sandbox/test/integration_test.rb
index d5ff1bdd..14ab6a2d 100644
--- a/test/sandbox/test/integration_test.rb
+++ b/test/sandbox/test/integration_test.rb
@@ -721,6 +721,24 @@ class IntegrationTest < ActionDispatch::IntegrationTest
 
     assert_raises ViewComponent::SystemTestControllerNefariousPathError do
       get "/_system_test_entrypoint?file=#{path}"
     end
   end
+
+  def test_unsafe_component
+    warnings = capture_warnings { get "/unsafe_component" }
+    assert_select("script", false)
+    assert(
+      warnings.any? { |warning| warning.include?("component rendered HTML-unsafe output") },
+      "Rendering UnsafeComponent did not emit an HTML safety warning"
+    )
+  end
+
+  def test_unsafe_postamble_component
+    warnings = capture_warnings { get "/unsafe_postamble_component" }
+    assert_select("script", false)
+    assert(
+      warnings.any? { |warning| warning.include?("component was provided an HTML-unsafe postamble") },
+      "Rendering UnsafePostambleComponent did not emit an HTML safety warning"
+    )
+  end
 end
diff --git a/test/sandbox/test/rendering_test.rb b/test/sandbox/test/rendering_test.rb
index cd985beb..f0962ddb 100644
--- a/test/sandbox/test/rendering_test.rb
+++ b/test/sandbox/test/rendering_test.rb
@@ -149,11 +149,13 @@ class RenderingTest < ViewComponent::TestCase
     assert_text("foo")
     assert_text("bar")
   end
 
   def test_render_jbuilder_template
-    render_inline(JbuilderComponent.new(message: "bar")) { "foo" }
+    with_request_url("/", format: :json) do
+      render_inline(JbuilderComponent.new(message: "bar")) { "foo" }
+    end
 
     assert_text("foo")
     assert_text("bar")
   end
 
@@ -1082,11 +1084,11 @@ class RenderingTest < ViewComponent::TestCase
 
     assert_text("Default")
   end
 
   def test_content_predicate_true
-    render_inline(ContentPredicateComponent.new.with_content("foo"))
+    render_inline(ContentPredicateComponent.new.with_content("foo".html_safe))
 
     assert_text("foo")
   end
 
   def test_content_security_policy_nonce
diff --git a/test/test_helper.rb b/test/test_helper.rb
index 9af2a43f..b4ba2b9b 100644
--- a/test/test_helper.rb
+++ b/test/test_helper.rb
@@ -177,5 +177,13 @@ def with_compiler_mode(mode)
   ViewComponent::Compiler.mode = mode
   yield
 ensure
   ViewComponent::Compiler.mode = previous_mode
 end
+
+def capture_warnings(&block)
+  [].tap do |warnings|
+    Kernel.stub(:warn, ->(msg) { warnings << msg }) do
+      block.call
+    end
+  end
+end

Vulnerable code:         name: simplecov-resultset-rails${{matrix.rails_version}}-ruby${{matrix.ruby_version}}
          render_template_for(@__vc_variant).to_s
          render_template_for(@__vc_variant).to_s + output_postamble
      ""
    def with_request_url(full_path, host: nil, method: nil)
    "Hello, "
    "World!"
      "Default"
    helpers.foo
    "<div>hello world</div>"
    @items.map { |c| render(c) }.join
    @message
    "Rendered"
    @variant.to_s
        "<div data-name='#{product.name}'><h1>#{product.name}</h1></div>"
    render_inline(JbuilderComponent.new(message: "bar")) { "foo" }
    render_inline(ContentPredicateComponent.new.with_content("foo"))
Fixed code:         name: simplecov-resultset-rails${{matrix.rails_version}}-ruby${{matrix.ruby_version}}-${{matrix.mode}}
ruby_version: 2.5
* Ensure HTML output safety.

    *Cameron Dutro*

          safe_render_template_for(@__vc_variant).to_s
          safe_render_template_for(@__vc_variant).to_s + safe_output_postamble
      @@default_output_postamble ||= "".html_safe
    def maybe_escape_html(text)
      return text if request && !request.format.html?
      return text if text.nil? || text.empty?

      if text.html_safe?
        text
      else
        yield
        html_escape(text)
      end
    end

    def safe_render_template_for(variant)
      if compiler.renders_template_for_variant?(variant)
        render_template_for(variant)
      else
        maybe_escape_html(render_template_for(variant)) do
          Kernel.warn("WARNING: The #{self.class} component rendered HTML-unsafe output. The output will be automatically escaped, but you may want to investigate.")
        end
      end
    end

    def safe_output_postamble
      maybe_escape_html(output_postamble) do
        Kernel.warn("WARNING: The #{self.class} component was provided an HTML-unsafe postamble. The postamble will be automatically escaped, but you may want to investigate.")
      end
    end

    def compiler
      @compiler ||= self.class.compiler
    end

      @variants_rendering_templates = Set.new
          @variants_rendering_templates << template[:variant]
    def renders_template_for_variant?(variant)
      @variants_rendering_templates.include?(variant)
    end

    def with_request_url(full_path, host: nil, method: nil, format: :html)
      old_request_format = vc_test_request.format.symbol
      vc_test_request.format = format
      vc_test_request.format = old_request_format
    "Hello, ".html_safe
    "World!".html_safe
      "Default".html_safe
    html_escape(helpers.foo)
    "<div>hello world</div>".html_safe
    @items.map { |c| render(c) }.join.html_safe
    html_escape(@message)
    "Rendered".html_safe
# frozen_string_literal: true

class UnsafeComponent < ViewComponent::Base
  def call
    user_input = "<script>alert('hello!')</script>"

    "<div>hello #{user_input}</div>"
  end
end
# frozen_string_literal: true

class UnsafePostambleComponent < ViewComponent::Base
  def call
    "<div>some content</div>".html_safe
  end

  def output_postamble
    "<script>alert('hello!')</script>"
  end
end
    html_escape(@variant.to_s)

  def unsafe_component
    render(UnsafeComponent.new)
  end

  def unsafe_postamble_component
    render(UnsafePostambleComponent.new)
  end
  get :unsafe_component, to: "integration_examples#unsafe_component"
  get :unsafe_postamble_component, to: "integration_examples#unsafe_postamble_component"
        "<div data-name='#{product.name}'><h1>#{product.name}</h1></div>".html_safe

  def test_unsafe_component
    warnings = capture_warnings { get "/unsafe_component" }
    assert_select("script", false)
    assert(
      warnings.any? { |warning| warning.include?("component rendered HTML-unsafe output") },
      "Rendering UnsafeComponent did not emit an HTML safety warning"
    )
  end

  def test_unsafe_postamble_component
    warnings = capture_warnings { get "/unsafe_postamble_component" }
    assert_select("script", false)
    assert(
      warnings.any? { |warning| warning.include?("component was provided an HTML-unsafe postamble") },
      "Rendering UnsafePostambleComponent did not emit an HTML safety warning"
    )
  end
    with_request_url("/", format: :json) do
      render_inline(JbuilderComponent.new(message: "bar")) { "foo" }
    end
    render_inline(ContentPredicateComponent.new.with_content("foo".html_safe))

def capture_warnings(&block)
  [].tap do |warnings|
    Kernel.stub(:warn, ->(msg) { warnings << msg }) do
      block.call
    end
  end
end