Commit 943ebda3 authored by Markus Koller's avatar Markus Koller

Disable caching for wiki attachments

These were served with `Content-Disposition: inline` in some situations,
which led to a Stored XSS attack using SVG files.

Workhorse has protections specifically against SVG files and will
rewrite the `Content-Disposition` header to `attachment`, but this
processing is skipped for cached 304 responses.

By disabling caching we force Workhorse to always rewrite this header.
parent 98e46c01
......@@ -56,7 +56,7 @@ module WikiActions
render 'show'
elsif file_blob
send_blob(wiki.repository, file_blob, allow_caching: container.public?)
send_blob(wiki.repository, file_blob)
elsif show_create_form?
# Assign a title to the WikiPage unless `id` is a randomly generated slug from #new
title = params[:id] unless params[:random_title].present?
......
---
title: Disable caching for wiki attachments
merge_request:
author:
type: security
......@@ -158,46 +158,18 @@ RSpec.shared_examples 'wiki controller actions' do
context 'when page is a file' do
include WikiHelpers
let(:id) { upload_file_to_wiki(container, user, file_name) }
where(:file_name) { ['dk.png', 'unsanitized.svg', 'git-cheat-sheet.pdf'] }
context 'when file is an image' do
let(:file_name) { 'dk.png' }
with_them do
let(:id) { upload_file_to_wiki(container, user, file_name) }
it 'delivers the image' do
it 'delivers the file with the correct headers' do
subject
expect(response.headers['Content-Disposition']).to match(/^inline/)
expect(response.headers[Gitlab::Workhorse::DETECT_HEADER]).to eq "true"
end
context 'when file is a svg' do
let(:file_name) { 'unsanitized.svg' }
it 'delivers the image' do
subject
expect(response.headers['Content-Disposition']).to match(/^inline/)
expect(response.headers[Gitlab::Workhorse::DETECT_HEADER]).to eq "true"
end
end
it_behaves_like 'project cache control headers' do
let(:project) { container }
end
end
context 'when file is a pdf' do
let(:file_name) { 'git-cheat-sheet.pdf' }
it 'sets the content type to sets the content response headers' do
subject
expect(response.headers['Content-Disposition']).to match(/^inline/)
expect(response.headers[Gitlab::Workhorse::DETECT_HEADER]).to eq "true"
end
it_behaves_like 'project cache control headers' do
let(:project) { container }
expect(response.headers[Gitlab::Workhorse::DETECT_HEADER]).to eq('true')
expect(response.cache_control[:public]).to be(false)
expect(response.cache_control[:extras]).to include('no-store')
end
end
end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment