From e081cc14fb3b41c0ac72aa189010ac1c7c4a8ccf Mon Sep 17 00:00:00 2001 From: Bianca Nenciu Date: Fri, 27 Sep 2024 12:55:03 +0300 Subject: [PATCH] SECURITY: Use different anon cache keys for XHR requests XHR requests are handled differently by the application and the responses do not have any preloaded data so the cache key needs to differntiate between those requests. --- lib/middleware/anonymous_cache.rb | 8 ++++- spec/requests/application_controller_spec.rb | 35 ++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/lib/middleware/anonymous_cache.rb b/lib/middleware/anonymous_cache.rb index 04e058c3c59..1b472aef734 100644 --- a/lib/middleware/anonymous_cache.rb +++ b/lib/middleware/anonymous_cache.rb @@ -156,8 +156,14 @@ module Middleware def cache_key return @cache_key if defined?(@cache_key) + # Rack `xhr?` performs a case sensitive comparison, but Rails `xhr?` + # performs a case insensitive comparison. We use the latter everywhere + # else in the application, so we should use it here as well. + is_xhr = @env["HTTP_X_REQUESTED_WITH"]&.casecmp("XMLHttpRequest") == 0 ? "t" : "f" + @cache_key = - +"ANON_CACHE_#{@env["HTTP_ACCEPT"]}_#{@env[Rack::RACK_URL_SCHEME]}_#{@env["HTTP_HOST"]}#{@env["REQUEST_URI"]}" + +"ANON_CACHE_#{is_xhr}_#{@env["HTTP_ACCEPT"]}_#{@env[Rack::RACK_URL_SCHEME]}_#{@env["HTTP_HOST"]}#{@env["REQUEST_URI"]}" + @cache_key << AnonymousCache.build_cache_key(self) @cache_key end diff --git a/spec/requests/application_controller_spec.rb b/spec/requests/application_controller_spec.rb index 539e8728825..f0ee085ae21 100644 --- a/spec/requests/application_controller_spec.rb +++ b/spec/requests/application_controller_spec.rb @@ -1256,6 +1256,41 @@ RSpec.describe ApplicationController do get "/", headers: { "HTTP_USER_AGENT" => "iam badcrawler" } expect(response.status).to eq(429) end + + context "with XHR requests" do + before { global_setting :anon_cache_store_threshold, 1 } + + def preloaded_data + response_html = Nokogiri::HTML5.fragment(response.body) + JSON.parse(response_html.css("#data-preloaded")[0]["data-preloaded"]) + end + + it "does not return the same preloaded data for XHR and non-XHR requests" do + # Request is stored in cache + get "/", headers: { "X-Requested-With" => "XMLHTTPrequest" } + expect(response.status).to eq(200) + expect(response.headers["X-Discourse-Cached"]).to eq("store") + expect(preloaded_data).not_to have_key("site") + + # Request is served from cache + get "/", headers: { "X-Requested-With" => "xmlhttprequest" } + expect(response.status).to eq(200) + expect(response.headers["X-Discourse-Cached"]).to eq("true") + expect(preloaded_data).not_to have_key("site") + + # Request is not served from cache because of different headers, but is stored + get "/" + expect(response.status).to eq(200) + expect(response.headers["X-Discourse-Cached"]).to eq("store") + expect(preloaded_data).to have_key("site") + + # Request is served from cache + get "/", headers: { "X-Requested-With" => "xmlhttprequest" } + expect(response.status).to eq(200) + expect(response.headers["X-Discourse-Cached"]).to eq("true") + expect(preloaded_data).not_to have_key("site") + end + end end end