From 58d7c38523a8627d1e5455067a992d30fd9fb80f Mon Sep 17 00:00:00 2001 From: Emi Matchu Date: Thu, 12 Sep 2024 15:59:18 -0700 Subject: [PATCH] Simplify CSP header for SWF asset embeds, to fix 502 for some assets Fun little bug: viewing the "Engulfed in Flames Effect" item was showing our "502 Bad Gateway" custom error page in the embed. This is because the Rails app was providing a `Content-Security-Policy` header value that was longer than nginx is configured by default to allow, so it was refusing the response, and showing the same 502 error as if the app hadn't responded at all. (We discovered this by opening `/var/log/nginx/error.log`, which explained this very clearly, ty~!) In this change, we no longer list every `images.neopets.com` asset, instead marking the entire domain as a valid image source for the SWF asset embed iframe. I don't _love_ this solution, I liked the property of specifying literally exactly the assets we allow! But I don't think there's any practical danger here, and it helps a *lot* for making this more reliable. (If we could have solved this reliably by increasing nginx's allowed response header size, I probably would've done that? But I researched a bit, and ultimately concluded that I don't trust other intermediary software like firewalls not to have the same issue. Let's not be pushing the limits of HTTP headers of all things!) --- app/controllers/swf_assets_controller.rb | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/app/controllers/swf_assets_controller.rb b/app/controllers/swf_assets_controller.rb index bd55a06a..43c0c898 100644 --- a/app/controllers/swf_assets_controller.rb +++ b/app/controllers/swf_assets_controller.rb @@ -12,6 +12,13 @@ class SwfAssetsController < ApplicationController helpers.image_url("favicon.png"), @swf_asset.image_url, *@swf_asset.canvas_movie_sprite_urls, + + # For images, `images.neopets.com` is a generally safe host to load + # from (shouldn't be a vulnerable site or exfiltration vector), and + # doing this can help make this header a *lot* shorter, which helps + # our nginx reverse proxy (and probably some clients) handle it. (For + # example, see asset `667993` for "Engulfed in Flames Effect".) + hosts: ["https://images.neopets.com"], ) } @@ -38,7 +45,14 @@ class SwfAssetsController < ApplicationController private - def src_list(*urls) - urls.filter(&:present?).map { |url| url.sub(/\?.*\z/, "") }.join(" ") + def src_list(*urls, hosts: []) + urls. + # Ignore any `nil`s that might arise + filter(&:present?). + # Remove query strings from URLs (they're invalid in CSPs) + map { |url| url.sub(/\?.*\z/, "") }. + # For the given `hosts`, remove all their specific URLs, and just list + # the host itself. + reject { |url| hosts.any? { |h| url.start_with? h } } + hosts end end