diff --git a/lib/html_sanitizer.rb b/lib/html_sanitizer.rb
index 8c6308a2b..f8b7a09ba 100644
--- a/lib/html_sanitizer.rb
+++ b/lib/html_sanitizer.rb
@@ -380,7 +380,28 @@ cleanup html string:
else
/[[:space:]]|\t|\n|\r/
end
- string.strip.gsub(blank_regex, '').gsub(%r{/\*.*?\*/}, '').gsub(//, '').gsub(/\[.+?\]/, '').delete("\u0000")
+ cleaned_string = string.strip.gsub(blank_regex, '').gsub(%r{/\*.*?\*/}, '').gsub(//, '').gsub(/\[.+?\]/, '').delete("\u0000")
+ sanitize_attachment_disposition(cleaned_string)
+ end
+
+ def self.sanitize_attachment_disposition(url)
+ uri = URI(url)
+ return url if uri.host != Setting.get('fqdn')
+
+ params = CGI.parse(uri.query || '')
+ if params.key?('disposition')
+ params['disposition'] = 'attachment'
+ end
+
+ uri.query = if params.blank?
+ nil
+ else
+ URI.encode_www_form(params)
+ end
+
+ uri.to_s
+ rescue URI::InvalidURIError
+ url
end
def self.url_same?(url_new, url_old)
diff --git a/test/unit/html_sanitizer_test.rb b/test/unit/html_sanitizer_test.rb
index e084a63cc..9c95a0dad 100644
--- a/test/unit/html_sanitizer_test.rb
+++ b/test/unit/html_sanitizer_test.rb
@@ -113,9 +113,26 @@ test 123
assert_equal(HtmlSanitizer.strict('
'), '')
assert_equal(HtmlSanitizer.strict(''), '')
assert_equal(HtmlSanitizer.strict(''), '')
+ assert_equal(HtmlSanitizer.strict(''), '')
assert_equal(HtmlSanitizer.strict('test'), 'test')
assert_equal(HtmlSanitizer.strict('test'), 'test')
assert_equal(HtmlSanitizer.strict('test'), 'test')
- end
+ api_path = Rails.configuration.api_path
+ http_type = Setting.get('http_type')
+ fqdn = Setting.get('fqdn')
+ attachment_url = "#{http_type}://#{fqdn}#{api_path}/ticket_attachment/239/986/1653"
+ attachment_url_good = "#{attachment_url}?disposition=attachment"
+ attachment_url_evil = "#{attachment_url}?disposition=inline"
+ assert_equal(HtmlSanitizer.strict("Evil link"), "Evil link")
+
+ assert_equal(HtmlSanitizer.strict("Good link"), "Good link")
+ assert_equal(HtmlSanitizer.strict("No disposition"), "No disposition")
+
+ different_fqdn_url = attachment_url_evil.gsub(fqdn, 'some.other.tld')
+ assert_equal(HtmlSanitizer.strict("Different FQDN"), "Different FQDN")
+
+ attachment_url_evil_other = "#{attachment_url}?disposition=some_other"
+ assert_equal(HtmlSanitizer.strict("Evil link"), "Evil link")
+ end
end