diff --git a/app/controllers/admin/themes_controller.rb b/app/controllers/admin/themes_controller.rb index c25c2c17677..26733c5e9a5 100644 --- a/app/controllers/admin/themes_controller.rb +++ b/app/controllers/admin/themes_controller.rb @@ -3,6 +3,8 @@ require "base64" class Admin::ThemesController < Admin::AdminController + MAX_REMOTE_LENGTH = 10_000 + skip_before_action :check_xhr, only: %i[show preview export] before_action :ensure_admin @@ -86,6 +88,12 @@ class Admin::ThemesController < Admin::AdminController render json: @theme.errors, status: :unprocessable_entity end elsif remote = params[:remote] + if remote.length > MAX_REMOTE_LENGTH + error = + I18n.t("themes.import_error.not_allowed_theme", { repo: remote[0..MAX_REMOTE_LENGTH] }) + return render_json_error(error, status: 422) + end + begin guardian.ensure_allowed_theme_repo_import!(remote.strip) rescue Discourse::InvalidAccess diff --git a/lib/url_helper.rb b/lib/url_helper.rb index 320f3babd28..432464798b2 100644 --- a/lib/url_helper.rb +++ b/lib/url_helper.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class UrlHelper + MAX_URL_LENGTH = 100_000 + # At the moment this handles invalid URLs that browser address bar accepts # where second # is not encoded # @@ -74,9 +76,10 @@ class UrlHelper end def self.normalized_encode(uri) - validated = nil url = uri.to_s + raise ArgumentError.new(:uri, "URL is too long") if url.length > MAX_URL_LENGTH + # Ideally we will jump straight to `Addressable::URI.normalized_encode`. However, # that implementation has some edge-case issues like https://github.com/sporkmonger/addressable/issues/472. # To temporaily work around those issues for the majority of cases, we try parsing with `::URI`. diff --git a/spec/lib/url_helper_spec.rb b/spec/lib/url_helper_spec.rb index e95f65c14fc..e98d5ac829d 100644 --- a/spec/lib/url_helper_spec.rb +++ b/spec/lib/url_helper_spec.rb @@ -164,6 +164,12 @@ RSpec.describe UrlHelper do "https://test.com/original/3X/b/5/575bcc2886bf7a39684b57ca90be85f7d399bbc7.png?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=AK8888999977/20200130/us-west-1/s3/aws4_request&X-Amz-Date=20200130T064355Z&X-Amz-Expires=15&X-Amz-SignedHeaders=host&X-Amz-Security-Token=blahblah+blahblah/blah//////////wEQA==&X-Amz-Signature=test" expect(UrlHelper.normalized_encode(presigned_url)).not_to eq(encoded_presigned_url) end + + it "raises error if too long" do + expect do UrlHelper.normalized_encode("https://#{"a" * 100_000}.com") end.to raise_error( + ArgumentError, + ) + end end describe "#local_cdn_url" do diff --git a/spec/requests/admin/themes_controller_spec.rb b/spec/requests/admin/themes_controller_spec.rb index 0b09e698e3b..5c494c2b395 100644 --- a/spec/requests/admin/themes_controller_spec.rb +++ b/spec/requests/admin/themes_controller_spec.rb @@ -241,6 +241,12 @@ RSpec.describe Admin::ThemesController do expect(response.status).to eq(422) end + it "fails to import with a failing status" do + post "/admin/themes/import.json", params: { remote: "https://#{"a" * 10_000}.com" } + + expect(response.status).to eq(422) + end + it "can lookup a private key by public key" do Discourse.redis.setex("ssh_key_abcdef", 1.hour, "rsa private key")