Ensure HTML output safety

[camertron]

What are you trying to accomplish?

This PR addresses a security concern affecting components that define a #call method. In most cases, HTML escaping is handled either by the template handler (ERB, Slim, HAML, etc) or by ActionView when the rendered string is appended to the output buffer. However, if a controller renders a component directly, the rendered string is not escaped and can be a vector for cross-site scripting (XSS) and similar attacks.

Consider the following component:

class UnsafeComponent < ApplicationComponent
  def initialize(user_defined_input)
    @user_defined_input = user_defined_input
  end

  def call
    "<div>#{@user_defined_input}</div>"
  end
end

If the component is rendered directly by the controller...

class MyController < ApplicationController
  def create
    render UnsafeComponent.new(params[:user_defined_input])
  end
end

...and params[:user_defined_input] is the string <script>alert("foo")</script>, the alert will be injected onto the page without being escaped, resulting in a browser alert. Since params[:user_defined_input] can contain arbitrary JavaScript code, cookies and other sensitive information can be exfiltrated to a 3rd-party.

What approach did you choose and why?

This PR ensures that rendered output is HTML escaped, and if it is not, prints a warning and escapes it automatically. It does this for output from #call methods, output generated by template handlers, and for postambles.

Finally, it examines the MIME type of the request and only performs automatic escaping when generating HTML. In other words, JSON generated by eg. jbuilder will not be automatically escaped.

Performance Impact

Warming up --------------------------------------
              render   366.000 i/100ms
Calculating -------------------------------------
              render      3.703k (± 6.1%) i/s -     36.966k in  10.021240s

Warming up --------------------------------------
         safe render   302.000 i/100ms
Calculating -------------------------------------
         safe render      3.522k (±11.5%) i/s -     34.730k in  10.005739s

Comparison:
              render:     3703.0 i/s
         safe render:     3522.1 i/s - same-ish: difference falls within error

Performance is impacted, but not to a significant degree 👍

[joelhawksley]

👏🏻

[reeganviljoen]

@camertron can we please add this as a security advisory so dependabot can pick this up

[camertron]

@reeganviljoen yes, that's the plan! I'll submit one as soon as this is merged and released 👍

[Spone]

What are your thoughts on backporting this fix to the v2 as well?

[BlakeWilliams]

LGTM!

[camertron]

@Spone we discussed that a bit internally and decided not to, but I could be persuaded otherwise.

[Spone]

@Spone we discussed that a bit internally and decided not to, but I could be persuaded otherwise.

Well, I have half a dozen Rails apps that cannot be deployed today because bundler-audit is a mandatory step in my deployment pipeline:

$ bundle exec bundle-audit check --update
Download ruby-advisory-db ...
Cloning into '/root/.local/share/ruby-advisory-db'...
ruby-advisory-db:
  advisories:	842 advisories
  last updated:	2024-01-07 12:48:57 -0800
  commit:	2bf8ea3b6c77df021a20db7cb63517dd4ca87bde
Name: view_component
Version: 2.82.0
CVE: CVE-2024-21636
GHSA: GHSA-wf2x-8w6j-qw37
Criticality: Medium
URL: https://github.com/ViewComponent/view_component/security/advisories/GHSA-wf2x-8w6j-qw37
Title: view_component Cross-site Scripting vulnerability
Solution: upgrade to >= 3.9.0
Vulnerabilities found!

I can either disable the bundler-audit check and open myself to vulnerabilities in other gems, or upgrade to ViewComponent 3.x, but given the slots breaking change, that would take hours if not days.

EDIT: I just figured out about the ignore configuration file for bundler-audit, but I think it's still not ideal that these Rails apps will stay exposed until they are upgraded to VC 3.

I'm pretty confident I'm not the only one in this case.