CVE-2023-37467: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting') (CWE-79)
value: Discourse is an open source discussion platform. Prior to version 3.1.0.beta7 of the `beta` and `tests-passed` branches, a CSP (Content Security Policy) nonce reuse vulnerability was discovered could allow cross-site scripting (XSS) attacks to bypass CSP protection for anonymous (i.e. unauthenticated) users. There are no known XSS vectors at the moment, but should one be discovered, this vulnerability would allow the XSS attack to bypass CSP and execute successfully. This vulnerability isn't applicable to logged-in users. Version 3.1.0.beta7 contains a patch. The stable branch doesn't have this vulnerability. A workaround to prevent the vulnerability is to disable Google Tag Manager, i.e., unset the `gtm container id` setting.
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: 2023-07-28T15:15Z
Repository: https://github.com/discourse/discourse
Language: Ruby
Files changed: app/helpers/application_helper.rb, app/views/common/_google_tag_manager_head.html.erb, config/application.rb, config/initializers/099-anon-cache.rb, lib/content_security_policy.rb, lib/content_security_policy/builder.rb, lib/content_security_policy/default.rb, lib/content_security_policy/middleware.rb, lib/middleware/anonymous_cache.rb, lib/middleware/gtm_script_nonce_injector.rb, spec/lib/content_security_policy_spec.rb, spec/lib/middleware/anonymous_cache_spec.rb, spec/lib/middleware/request_tracker_spec.rb, spec/rails_helper.rb, spec/requests/application_controller_spec.rb
Commit date: 2023-07-28 12:53:44 +0100
Commit message: SECURITY: Don't reuse CSP nonce between anonymous requests

Diff: diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 7e3c903e3d5e..de0a22b0b12a 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -62,12 +62,14 @@ module ApplicationHelper
 
   def google_tag_manager_json
     google_universal_analytics_json
   end
 
-  def self.google_tag_manager_nonce(env)
-    env[:discourse_content_security_policy_nonce] ||= SecureRandom.hex
+  def google_tag_manager_nonce_placeholder
+    placeholder = "[[csp_nonce_placeholder_#{SecureRandom.hex}]]"
+    response.headers["Discourse-GTM-Nonce-Placeholder"] = placeholder
+    placeholder
   end
 
   def shared_session_key
     if SiteSetting.long_polling_base_url != "/" && current_user
       sk = "shared_session_key"
diff --git a/app/views/common/_google_tag_manager_head.html.erb b/app/views/common/_google_tag_manager_head.html.erb
index 26790ca3db75..56a77cfeed6d 100644
--- a/app/views/common/_google_tag_manager_head.html.erb
+++ b/app/views/common/_google_tag_manager_head.html.erb
@@ -1,6 +1,6 @@
 <meta id="data-google-tag-manager"
   data-data-layer="<%= google_tag_manager_json %>"
-  data-nonce="<%= ApplicationHelper.google_tag_manager_nonce(request.env) %>"
+  data-nonce="<%= google_tag_manager_nonce_placeholder %>"
   data-container-id="<%= SiteSetting.gtm_container_id %>" />
 
 <%= preload_script 'google-tag-manager' %>
diff --git a/config/application.rb b/config/application.rb
index 0962bccb6cac..9664112df22f 100644
--- a/config/application.rb
+++ b/config/application.rb
@@ -165,10 +165,13 @@ module Discourse
 
     require "content_security_policy/middleware"
     config.middleware.swap ActionDispatch::ContentSecurityPolicy::Middleware,
                            ContentSecurityPolicy::Middleware
 
+    require "middleware/gtm_script_nonce_injector"
+    config.middleware.insert_after(ActionDispatch::Flash, Middleware::GtmScriptNonceInjector)
+
     require "middleware/discourse_public_exceptions"
     config.exceptions_app = Middleware::DiscoursePublicExceptions.new(Rails.public_path)
 
     require "discourse_js_processor"
     require "discourse_sourcemapping_url_processor"
diff --git a/config/initializers/099-anon-cache.rb b/config/initializers/099-anon-cache.rb
index ee5e240d93a4..518f3d8685c9 100644
--- a/config/initializers/099-anon-cache.rb
+++ b/config/initializers/099-anon-cache.rb
@@ -4,12 +4,13 @@ require "middleware/anonymous_cache"
 
 enabled =
   if Rails.configuration.respond_to?(:enable_anon_caching)
     Rails.configuration.enable_anon_caching
   else
-    Rails.env.production?
+    Rails.env.production? || Rails.env.test?
   end
 
 if !ENV["DISCOURSE_DISABLE_ANON_CACHE"] && enabled
   # in an ideal world this is position 0, but mobile detection uses ... session and request and params
-  Rails.configuration.middleware.insert_after ActionDispatch::Flash, Middleware::AnonymousCache
+  Rails.configuration.middleware.insert_after Middleware::GtmScriptNonceInjector,
+                                              Middleware::AnonymousCache
 end
diff --git a/lib/content_security_policy.rb b/lib/content_security_policy.rb
index 53cf16d6103a..107dc0437dfb 100644
--- a/lib/content_security_policy.rb
+++ b/lib/content_security_policy.rb
@@ -2,17 +2,17 @@
 require "content_security_policy/builder"
 require "content_security_policy/extension"
 
 class ContentSecurityPolicy
   class << self
-    def policy(theme_id = nil, env: {}, base_url: Discourse.base_url, path_info: "/")
-      new.build(theme_id, env: env, base_url: base_url, path_info: path_info)
+    def policy(theme_id = nil, base_url: Discourse.base_url, path_info: "/")
+      new.build(theme_id, base_url: base_url, path_info: path_info)
     end
   end
 
-  def build(theme_id, env: {}, base_url:, path_info: "/")
-    builder = Builder.new(base_url: base_url, env: env)
+  def build(theme_id, base_url:, path_info: "/")
+    builder = Builder.new(base_url: base_url)
 
     Extension.theme_extensions(theme_id).each { |extension| builder << extension }
     Extension.plugin_extensions.each { |extension| builder << extension }
     builder << Extension.site_setting_extension
     builder << Extension.path_specific_extension(path_info)
diff --git a/lib/content_security_policy/builder.rb b/lib/content_security_policy/builder.rb
index ddcb9db67eb4..e23f55111e2a 100644
--- a/lib/content_security_policy/builder.rb
+++ b/lib/content_security_policy/builder.rb
@@ -23,12 +23,12 @@ class ContentSecurityPolicy
       media_src
       prefetch_src
       style_src
     ].freeze
 
-    def initialize(base_url:, env: {})
-      @directives = Default.new(base_url: base_url, env: env).directives
+    def initialize(base_url:)
+      @directives = Default.new(base_url: base_url).directives
       @base_url = base_url
     end
 
     def <<(extension)
       return unless valid_extension?(extension)
diff --git a/lib/content_security_policy/default.rb b/lib/content_security_policy/default.rb
index ff3767c51fe4..a4e6f70aefc9 100644
--- a/lib/content_security_policy/default.rb
+++ b/lib/content_security_policy/default.rb
@@ -3,13 +3,12 @@ require "content_security_policy"
 
 class ContentSecurityPolicy
   class Default
     attr_reader :directives
 
-    def initialize(base_url:, env: {})
+    def initialize(base_url:)
       @base_url = base_url
-      @env = env
       @directives =
         {}.tap do |directives|
           directives[:upgrade_insecure_requests] = [] if SiteSetting.force_https
           directives[:base_uri] = [:self]
           directives[:object_src] = [:none]
@@ -84,11 +83,10 @@ class ContentSecurityPolicy
         if SiteSetting.ga_universal_tracking_code.present? && SiteSetting.ga_version == "v4_gtag"
           sources << "https://www.googletagmanager.com/gtag/js"
         end
         if SiteSetting.gtm_container_id.present?
           sources << "https://www.googletagmanager.com/gtm.js"
-          sources << "'nonce-#{ApplicationHelper.google_tag_manager_nonce(@env)}'"
         end
 
         sources << "'#{SplashScreenHelper.fingerprint}'" if SiteSetting.splash_screen
       end
     end
diff --git a/lib/content_security_policy/middleware.rb b/lib/content_security_policy/middleware.rb
index 9c729511cab8..79ec0834275c 100644
--- a/lib/content_security_policy/middleware.rb
+++ b/lib/content_security_policy/middleware.rb
@@ -19,17 +19,15 @@ class ContentSecurityPolicy
 
       theme_id = env[:resolved_theme_id]
 
       headers["Content-Security-Policy"] = policy(
         theme_id,
-        env: env,
         base_url: base_url,
         path_info: env["PATH_INFO"],
       ) if SiteSetting.content_security_policy
       headers["Content-Security-Policy-Report-Only"] = policy(
         theme_id,
-        env: env,
         base_url: base_url,
         path_info: env["PATH_INFO"],
       ) if SiteSetting.content_security_policy_report_only
 
       response
diff --git a/lib/middleware/anonymous_cache.rb b/lib/middleware/anonymous_cache.rb
index 7f1fbec9a4d3..d41069c92e02 100644
--- a/lib/middleware/anonymous_cache.rb
+++ b/lib/middleware/anonymous_cache.rb
@@ -41,10 +41,25 @@ module Middleware
 
     def self.anon_cache(env, duration)
       env["ANON_CACHE_DURATION"] = duration
     end
 
+    def self.clear_all_cache!
+      if Rails.env.production?
+        raise "for perf reasons, clear_all_cache! cannot be used in production."
+      end
+      Discourse.redis.keys("ANON_CACHE_*").each { |k| Discourse.redis.del(k) }
+    end
+
+    def self.disable_anon_cache
+      @@disabled = true
+    end
+
+    def self.enable_anon_cache
+      @@disabled = false
+    end
+
     # This gives us an API to insert anonymous cache segments
     class Helper
       RACK_SESSION = "rack.session"
       USER_AGENT = "HTTP_USER_AGENT"
       ACCEPT_ENCODING = "HTTP_ACCEPT_ENCODING"
@@ -230,11 +245,14 @@ module Middleware
 
         false
       end
 
       def cacheable?
-        !!(!has_auth_cookie? && get? && no_cache_bypass)
+        !!(
+          GlobalSetting.anon_cache_store_threshold > 0 && !has_auth_cookie? && get? &&
+            no_cache_bypass
+        )
       end
 
       def compress(val)
         if val && GlobalSetting.compress_anon_cache
           require "lz4-ruby" if !defined?(LZ4)
@@ -324,10 +342,12 @@ module Middleware
     end
 
     PAYLOAD_INVALID_REQUEST_METHODS = %w[GET HEAD]
 
     def call(env)
+      return @app.call(env) if defined?(@@disabled) && @@disabled
+
       if PAYLOAD_INVALID_REQUEST_METHODS.include?(env[Rack::REQUEST_METHOD]) &&
            env[Rack::RACK_INPUT].size > 0
         return 413, { "Cache-Control" => "private, max-age=0, must-revalidate" }, []
       end
 
diff --git a/lib/middleware/gtm_script_nonce_injector.rb b/lib/middleware/gtm_script_nonce_injector.rb
new file mode 100644
index 000000000000..b7fc3fda334e
--- /dev/null
+++ b/lib/middleware/gtm_script_nonce_injector.rb
@@ -0,0 +1,26 @@
+# frozen_string_literal: true
+
+module Middleware
+  class GtmScriptNonceInjector
+    def initialize(app, settings = {})
+      @app = app
+    end
+
+    def call(env)
+      status, headers, response = @app.call(env)
+
+      if nonce_placeholder = headers.delete("Discourse-GTM-Nonce-Placeholder")
+        nonce = SecureRandom.hex
+        parts = []
+        response.each { |part| parts << part.to_s.sub(nonce_placeholder, nonce) }
+        %w[Content-Security-Policy Content-Security-Policy-Report-Only].each do |name|
+          next if headers[name].blank?
+          headers[name] = headers[name].sub("script-src ", "script-src 'nonce-#{nonce}' ")
+        end
+        [status, headers, parts]
+      else
+        [status, headers, response]
+      end
+    end
+  end
+end
diff --git a/spec/lib/content_security_policy_spec.rb b/spec/lib/content_security_policy_spec.rb
index 87ea6713836b..338a2e1e0851 100644
--- a/spec/lib/content_security_policy_spec.rb
+++ b/spec/lib/content_security_policy_spec.rb
@@ -104,11 +104,13 @@ RSpec.describe ContentSecurityPolicy do
     it "allowlists Google Tag Manager when integrated" do
       SiteSetting.gtm_container_id = "GTM-ABCDEF"
 
       script_srcs = parse(policy)["script-src"]
       expect(script_srcs).to include("https://www.googletagmanager.com/gtm.js")
-      expect(script_srcs.to_s).to include("nonce-")
+      # nonce is added by the GtmScriptNonceInjector middleware to prevent the
+      # nonce from getting cached by AnonymousCache
+      expect(script_srcs.to_s).not_to include("nonce-")
     end
 
     it "allowlists CDN assets when integrated" do
       set_cdn_url("https://cdn.com")
 
diff --git a/spec/lib/middleware/anonymous_cache_spec.rb b/spec/lib/middleware/anonymous_cache_spec.rb
index 2369f8210448..33b52642daa4 100644
--- a/spec/lib/middleware/anonymous_cache_spec.rb
+++ b/spec/lib/middleware/anonymous_cache_spec.rb
@@ -1,10 +1,12 @@
 # frozen_string_literal: true
 
 RSpec.describe Middleware::AnonymousCache do
   let(:middleware) { Middleware::AnonymousCache.new(lambda { |_| [200, {}, []] }) }
 
+  before { Middleware::AnonymousCache.enable_anon_cache }
+
   def env(opts = {})
     create_request_env(path: opts.delete(:path) || "http://test.com/path?bla=1").merge(opts)
   end
 
   describe Middleware::AnonymousCache::Helper do
diff --git a/spec/lib/middleware/request_tracker_spec.rb b/spec/lib/middleware/request_tracker_spec.rb
index d3cc4f2798e6..a2ee21de7df0 100644
--- a/spec/lib/middleware/request_tracker_spec.rb
+++ b/spec/lib/middleware/request_tracker_spec.rb
@@ -675,10 +675,12 @@ RSpec.describe Middleware::RequestTracker do
     before { Middleware::RequestTracker.register_detailed_request_logger(logger) }
 
     after { Middleware::RequestTracker.unregister_detailed_request_logger(logger) }
 
     it "can report data from anon cache" do
+      Middleware::AnonymousCache.enable_anon_cache
+
       cache = Middleware::AnonymousCache.new(app([200, {}, ["i am a thing"]]))
       tracker = Middleware::RequestTracker.new(cache)
 
       uri = "/path?#{SecureRandom.hex}"
 
diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb
index 947e63b1d0b4..63e010de110a 100644
--- a/spec/rails_helper.rb
+++ b/spec/rails_helper.rb
@@ -147,10 +147,12 @@ module TestSetup
 
     # Don't queue badge grant in test mode
     BadgeGranter.disable_queue
 
     OmniAuth.config.test_mode = false
+
+    Middleware::AnonymousCache.disable_anon_cache
   end
 end
 
 TestProf::BeforeAll.configure do |config|
   config.after(:begin) do
diff --git a/spec/requests/application_controller_spec.rb b/spec/requests/application_controller_spec.rb
index a2f461fe8680..a2d86582990a 100644
--- a/spec/requests/application_controller_spec.rb
+++ b/spec/requests/application_controller_spec.rb
@@ -635,40 +635,67 @@ RSpec.describe ApplicationController do
       expect(response.headers).to_not include("Content-Security-Policy-Report-Only")
     end
 
     it "when GTM is enabled it adds the same nonce to the policy and the GTM tag" do
       SiteSetting.content_security_policy = true
+      SiteSetting.content_security_policy_report_only = true
       SiteSetting.gtm_container_id = "GTM-ABCDEF"
 
       get "/latest"
 
-      expect(response.headers).to include("Content-Security-Policy")
       script_src = parse(response.headers["Content-Security-Policy"])["script-src"]
+      report_only_script_src =
+        parse(response.headers["Content-Security-Policy-Report-Only"])["script-src"]
+
       nonce = extract_nonce_from_script_src(script_src)
+      report_only_nonce = extract_nonce_from_script_src(report_only_script_src)
+
+      expect(nonce).to eq(report_only_nonce)
 
       gtm_meta_tag = Nokogiri::HTML5.fragment(response.body).css("#data-google-tag-manager").first
       expect(gtm_meta_tag["data-nonce"]).to eq(nonce)
     end
 
-    it "doesn't reuse CSP nonces between requests" do
+    it "doesn't reuse nonces between requests" do
+      global_setting :anon_cache_store_threshold, 1
+      Middleware::AnonymousCache.enable_anon_cache
+      Middleware::AnonymousCache.clear_all_cache!
+
       SiteSetting.content_security_policy = true
+      SiteSetting.content_security_policy_report_only = true
       SiteSetting.gtm_container_id = "GTM-ABCDEF"
 
       get "/latest"
 
-      expect(response.headers).to include("Content-Security-Policy")
+      expect(response.headers["X-Discourse-Cached"]).to eq("store")
+      expect(response.headers).not_to include("Discourse-GTM-Nonce-Placeholder")
+
       script_src = parse(response.headers["Content-Security-Policy"])["script-src"]
+      report_only_script_src =
+        parse(response.headers["Content-Security-Policy-Report-Only"])["script-src"]
+
       first_nonce = extract_nonce_from_script_src(script_src)
+      first_report_only_nonce = extract_nonce_from_script_src(report_only_script_src)
+
+      expect(first_nonce).to eq(first_report_only_nonce)
 
       gtm_meta_tag = Nokogiri::HTML5.fragment(response.body).css("#data-google-tag-manager").first
       expect(gtm_meta_tag["data-nonce"]).to eq(first_nonce)
 
       get "/latest"
 
-      expect(response.headers).to include("Content-Security-Policy")
+      expect(response.headers["X-Discourse-Cached"]).to eq("true")
+      expect(response.headers).not_to include("Discourse-GTM-Nonce-Placeholder")
+
       script_src = parse(response.headers["Content-Security-Policy"])["script-src"]
+      report_only_script_src =
+        parse(response.headers["Content-Security-Policy-Report-Only"])["script-src"]
+
       second_nonce = extract_nonce_from_script_src(script_src)
+      second_report_only_nonce = extract_nonce_from_script_src(report_only_script_src)
+
+      expect(second_nonce).to eq(second_report_only_nonce)
 
       expect(first_nonce).not_to eq(second_nonce)
       gtm_meta_tag = Nokogiri::HTML5.fragment(response.body).css("#data-google-tag-manager").first
       expect(gtm_meta_tag["data-nonce"]).to eq(second_nonce)
     end

Vulnerable code:   def self.google_tag_manager_nonce(env)
    env[:discourse_content_security_policy_nonce] ||= SecureRandom.hex
  data-nonce="<%= ApplicationHelper.google_tag_manager_nonce(request.env) %>"
    Rails.env.production?
  Rails.configuration.middleware.insert_after ActionDispatch::Flash, Middleware::AnonymousCache
    def policy(theme_id = nil, env: {}, base_url: Discourse.base_url, path_info: "/")
      new.build(theme_id, env: env, base_url: base_url, path_info: path_info)
  def build(theme_id, env: {}, base_url:, path_info: "/")
    builder = Builder.new(base_url: base_url, env: env)
    def initialize(base_url:, env: {})
      @directives = Default.new(base_url: base_url, env: env).directives
    def initialize(base_url:, env: {})
      @env = env
          sources << "'nonce-#{ApplicationHelper.google_tag_manager_nonce(@env)}'"
        env: env,
        env: env,
        !!(!has_auth_cookie? && get? && no_cache_bypass)
      expect(script_srcs.to_s).to include("nonce-")
      expect(response.headers).to include("Content-Security-Policy")
    it "doesn't reuse CSP nonces between requests" do
      expect(response.headers).to include("Content-Security-Policy")
      expect(response.headers).to include("Content-Security-Policy")
Fixed code:   def google_tag_manager_nonce_placeholder
    placeholder = "[[csp_nonce_placeholder_#{SecureRandom.hex}]]"
    response.headers["Discourse-GTM-Nonce-Placeholder"] = placeholder
    placeholder
  data-nonce="<%= google_tag_manager_nonce_placeholder %>"
    require "middleware/gtm_script_nonce_injector"
    config.middleware.insert_after(ActionDispatch::Flash, Middleware::GtmScriptNonceInjector)

    Rails.env.production? || Rails.env.test?
  Rails.configuration.middleware.insert_after Middleware::GtmScriptNonceInjector,
                                              Middleware::AnonymousCache
    def policy(theme_id = nil, base_url: Discourse.base_url, path_info: "/")
      new.build(theme_id, base_url: base_url, path_info: path_info)
  def build(theme_id, base_url:, path_info: "/")
    builder = Builder.new(base_url: base_url)
    def initialize(base_url:)
      @directives = Default.new(base_url: base_url).directives
    def initialize(base_url:)
    def self.clear_all_cache!
      if Rails.env.production?
        raise "for perf reasons, clear_all_cache! cannot be used in production."
      end
      Discourse.redis.keys("ANON_CACHE_*").each { |k| Discourse.redis.del(k) }
    end

    def self.disable_anon_cache
      @@disabled = true
    end

    def self.enable_anon_cache
      @@disabled = false
    end

        !!(
          GlobalSetting.anon_cache_store_threshold > 0 && !has_auth_cookie? && get? &&
            no_cache_bypass
        )
      return @app.call(env) if defined?(@@disabled) && @@disabled

# frozen_string_literal: true

module Middleware
  class GtmScriptNonceInjector
    def initialize(app, settings = {})
      @app = app
    end

    def call(env)
      status, headers, response = @app.call(env)

      if nonce_placeholder = headers.delete("Discourse-GTM-Nonce-Placeholder")
        nonce = SecureRandom.hex
        parts = []
        response.each { |part| parts << part.to_s.sub(nonce_placeholder, nonce) }
        %w[Content-Security-Policy Content-Security-Policy-Report-Only].each do |name|
          next if headers[name].blank?
          headers[name] = headers[name].sub("script-src ", "script-src 'nonce-#{nonce}' ")
        end
        [status, headers, parts]
      else
        [status, headers, response]
      end
    end
  end
end
      # nonce is added by the GtmScriptNonceInjector middleware to prevent the
      # nonce from getting cached by AnonymousCache
      expect(script_srcs.to_s).not_to include("nonce-")
  before { Middleware::AnonymousCache.enable_anon_cache }

      Middleware::AnonymousCache.enable_anon_cache


    Middleware::AnonymousCache.disable_anon_cache
      SiteSetting.content_security_policy_report_only = true
      report_only_script_src =
        parse(response.headers["Content-Security-Policy-Report-Only"])["script-src"]

      report_only_nonce = extract_nonce_from_script_src(report_only_script_src)

      expect(nonce).to eq(report_only_nonce)
    it "doesn't reuse nonces between requests" do
      global_setting :anon_cache_store_threshold, 1
      Middleware::AnonymousCache.enable_anon_cache
      Middleware::AnonymousCache.clear_all_cache!

      SiteSetting.content_security_policy_report_only = true
      expect(response.headers["X-Discourse-Cached"]).to eq("store")
      expect(response.headers).not_to include("Discourse-GTM-Nonce-Placeholder")

      report_only_script_src =
        parse(response.headers["Content-Security-Policy-Report-Only"])["script-src"]

      first_report_only_nonce = extract_nonce_from_script_src(report_only_script_src)

      expect(first_nonce).to eq(first_report_only_nonce)
      expect(response.headers["X-Discourse-Cached"]).to eq("true")
      expect(response.headers).not_to include("Discourse-GTM-Nonce-Placeholder")

      report_only_script_src =
        parse(response.headers["Content-Security-Policy-Report-Only"])["script-src"]

      second_report_only_nonce = extract_nonce_from_script_src(report_only_script_src)

      expect(second_nonce).to eq(second_report_only_nonce)