replace the upload type whitelist with a sanitizer

This commit is contained in:
Régis Hanol
2017-05-18 12:13:13 +02:00
parent 8e5b0c79ae
commit 13e489b4ca
4 changed files with 12 additions and 19 deletions

View File

@ -5,9 +5,8 @@ class UploadsController < ApplicationController
skip_before_filter :preload_json, :check_xhr, :redirect_to_login_if_required, only: [:show] skip_before_filter :preload_json, :check_xhr, :redirect_to_login_if_required, only: [:show]
def create def create
type = params.require(:type) # 50 characters ought to be enough for the upload type
type = params.require(:type).parameterize("_")[0..50]
raise Discourse::InvalidAccess.new unless Upload::UPLOAD_TYPES.include?(type)
if type == "avatar" && (SiteSetting.sso_overrides_avatar || !SiteSetting.allow_uploaded_avatars) if type == "avatar" && (SiteSetting.sso_overrides_avatar || !SiteSetting.allow_uploaded_avatars)
return render json: failed_json, status: 422 return render json: failed_json, status: 422

View File

@ -21,9 +21,6 @@ class Upload < ActiveRecord::Base
validates_with ::Validators::UploadValidator validates_with ::Validators::UploadValidator
CROPPED_TYPES ||= %w{avatar card_background custom_emoji profile_background}.each(&:freeze)
UPLOAD_TYPES ||= CROPPED_TYPES + %w{composer category_logo category_background wizard_logo_url wizard_logo_small_url wizard_favicon_url wizard_apple_touch_icon_url}.each(&:freeze)
def thumbnail(width = self.width, height = self.height) def thumbnail(width = self.width, height = self.height)
optimized_images.find_by(width: width, height: height) optimized_images.find_by(width: width, height: height)
end end

View File

@ -5,10 +5,12 @@ class UploadCreator
TYPES_CONVERTED_TO_JPEG ||= %i{bmp png} TYPES_CONVERTED_TO_JPEG ||= %i{bmp png}
TYPES_TO_CROP ||= %w{avatar card_background custom_emoji profile_background}.each(&:freeze)
WHITELISTED_SVG_ELEMENTS ||= %w{ WHITELISTED_SVG_ELEMENTS ||= %w{
circle clippath defs ellipse g line linearGradient path polygon polyline circle clippath defs ellipse g line linearGradient path polygon polyline
radialGradient rect stop svg text textpath tref tspan use radialGradient rect stop svg text textpath tref tspan use
} }.each(&:freeze)
# Available options # Available options
# - type (string) # - type (string)
@ -177,7 +179,7 @@ class UploadCreator
end end
def should_crop? def should_crop?
Upload::CROPPED_TYPES.include?(@opts[:type]) TYPES_TO_CROP.include?(@opts[:type])
end end
def crop! def crop!

View File

@ -33,18 +33,13 @@ describe UploadsController do
}) })
end end
it 'fails if type is invalid' do it 'expects a type' do
xhr :post, :create, file: logo, type: "invalid type cause has space" expect { xhr :post, :create, file: logo }.to raise_error(ActionController::ParameterMissing)
expect(response.status).to eq 403 end
xhr :post, :create, file: logo, type: "\\invalid" it 'parameterize the type' do
expect(response.status).to eq 403 subject.expects(:create_upload).with(logo, nil, "super_long_type_with_charssuper_long_type_with_char")
xhr :post, :create, file: logo, type: "super \# long \//\\ type with \\. $%^&*( chars" * 5
xhr :post, :create, file: logo, type: "invalid."
expect(response.status).to eq 403
xhr :post, :create, file: logo, type: "toolong"*100
expect(response.status).to eq 403
end end
it 'is successful with an image' do it 'is successful with an image' do