From adfa7937310b77c65b505496ab1cd379e51d94b0 Mon Sep 17 00:00:00 2001 From: Dan Ungureanu Date: Wed, 11 Dec 2019 15:21:41 +0200 Subject: [PATCH] SECURITY: Ensure only image uploads can be inlined This prevents malicious files (for example special crafted XMLs) to be used in XSS attacks. --- app/controllers/uploads_controller.rb | 6 +++--- spec/requests/uploads_controller_spec.rb | 12 ++++++++++++ 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb index caa6c8987fa..0436c8fcc79 100644 --- a/app/controllers/uploads_controller.rb +++ b/app/controllers/uploads_controller.rb @@ -207,10 +207,10 @@ class UploadsController < ApplicationController content_type: MiniMime.lookup_by_filename(upload.original_filename)&.content_type } - if params[:inline] - opts[:disposition] = "inline" - elsif !FileHelper.is_supported_image?(upload.original_filename) + if !FileHelper.is_supported_image?(upload.original_filename) opts[:disposition] = "attachment" + elsif params[:inline] + opts[:disposition] = "inline" end file_path = Discourse.store.path_for(upload) diff --git a/spec/requests/uploads_controller_spec.rb b/spec/requests/uploads_controller_spec.rb index b2d877d9f35..512d419a22b 100644 --- a/spec/requests/uploads_controller_spec.rb +++ b/spec/requests/uploads_controller_spec.rb @@ -305,6 +305,18 @@ describe UploadsController do end describe "#show_short" do + it 'inlines only supported image files' do + upload = upload_file("smallest.png") + get upload.short_path, params: { inline: true } + expect(response.header['Content-Type']).to eq('image/png') + expect(response.header['Content-Disposition']).to include('inline;') + + upload.update!(original_filename: "test.xml") + get upload.short_path, params: { inline: true } + expect(response.header['Content-Type']).to eq('application/xml') + expect(response.header['Content-Disposition']).to include('attachment;') + end + describe "local store" do fab!(:image_upload) { upload_file("smallest.png") }