<% if attachment.preferences && attachment.preferences['Content-Type'] && @ContentTypeIcon(attachment.preferences['Content-Type']): %>
<% if @canPreview(attachment.preferences['Content-Type']): %>
-
 %>/ticket_attachment/<%= @article.ticket_id %>/<%= @article.id %>/<%= attachment.id %>)
+
 %>/ticket_attachment/<%= @article.ticket_id %>/<%= @article.id %>/<%= attachment.id %>?view=preview)
<% else: %>
<%- @Icon( @ContentTypeIcon(attachment.preferences['Content-Type']) ) %>
<% end %>
diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb
index 4e4be2efb..ee2944184 100644
--- a/app/controllers/sessions_controller.rb
+++ b/app/controllers/sessions_controller.rb
@@ -2,7 +2,7 @@
class SessionsController < ApplicationController
prepend_before_action :authentication_check, only: %i[switch_to_user list delete]
- skip_before_action :verify_csrf_token, only: %i[create show destroy create_omniauth failure_omniauth create_sso]
+ skip_before_action :verify_csrf_token, only: %i[show destroy create_omniauth failure_omniauth create_sso]
# "Create" a login, aka "log the user in"
def create
diff --git a/app/controllers/ticket_articles_controller.rb b/app/controllers/ticket_articles_controller.rb
index 681ffb17b..138a23fdd 100644
--- a/app/controllers/ticket_articles_controller.rb
+++ b/app/controllers/ticket_articles_controller.rb
@@ -255,8 +255,21 @@ class TicketArticlesController < ApplicationController
disposition = sanitized_disposition
+ content = nil
+ if params[:view].present? && file.preferences[:resizable] == true
+ if file.preferences[:content_inline] == true && params[:view] == 'inline'
+ content = file.content_inline
+ elsif file.preferences[:content_preview] == true && params[:view] == 'preview'
+ content = file.content_preview
+ end
+ end
+
+ if content.blank?
+ content = file.content
+ end
+
send_data(
- file.content,
+ content,
filename: file.filename,
type: file.preferences['Content-Type'] || file.preferences['Mime-Type'] || 'application/octet-stream',
disposition: disposition
diff --git a/app/models/organization.rb b/app/models/organization.rb
index d772fea1b..0b35e24af 100644
--- a/app/models/organization.rb
+++ b/app/models/organization.rb
@@ -7,6 +7,7 @@ class Organization < ApplicationModel
include HasHistory
include HasSearchIndexBackend
include CanCsvImport
+ include ChecksHtmlSanitized
include Organization::ChecksAccess
include Organization::Assets
@@ -22,6 +23,8 @@ class Organization < ApplicationModel
activity_stream_permission 'admin.role'
+ sanitized_html :note
+
private
def domain_cleanup
diff --git a/app/models/store.rb b/app/models/store.rb
index 5a79c1966..4827b2c5a 100644
--- a/app/models/store.rb
+++ b/app/models/store.rb
@@ -34,7 +34,7 @@ returns
=end
def self.add(data)
- data = data.stringify_keys
+ data.deep_stringify_keys!
# lookup store_object.id
store_object = Store::Object.create_if_not_exists(name: data['object'])
@@ -50,9 +50,34 @@ returns
data.delete('data')
data.delete('object')
+ data['preferences'] ||= {}
+ ['Mime-Type', 'Content-Type', 'mime_type', 'content_type'].each do |key|
+ next if data['preferences'][key].blank?
+ next if !data['preferences'][key].match(%r{image/(jpeg|jpg|png)}i)
+
+ data['preferences']['resizable'] = true
+ break
+ end
+
# store meta data
store = Store.create!(data)
+ begin
+ if store.preferences[:resizable] == true
+ if store.content_preview(silence: true)
+ store.preferences[:content_preview] = true
+ end
+ if store.content_inline(silence: true)
+ store.preferences[:content_inline] = true
+ end
+ store.save!
+ end
+ rescue => e
+ logger.error e
+ store.preferences[:resizable] = false
+ store.save!
+ end
+
store
end
@@ -165,6 +190,52 @@ returns
=begin
+get content of file in preview size
+
+ store = Store.find(store_id)
+ content_as_string = store.content_preview
+
+returns
+
+ content_as_string
+
+=end
+
+ def content_preview(options = {})
+ file = Store::File.find_by(id: store_file_id)
+ if !file
+ raise "No such file #{store_file_id}!"
+ end
+ raise 'Unable to generate preview' if options[:silence] != true && preferences[:content_preview] != true
+
+ image_resize(file.content, 200)
+ end
+
+=begin
+
+get content of file in inline size
+
+ store = Store.find(store_id)
+ content_as_string = store.content_inline
+
+returns
+
+ content_as_string
+
+=end
+
+ def content_inline(options = {})
+ file = Store::File.find_by(id: store_file_id)
+ if !file
+ raise "No such file #{store_file_id}!"
+ end
+ raise 'Unable to generate inline' if options[:silence] != true && preferences[:content_inline] != true
+
+ image_resize(file.content, 1800)
+ end
+
+=begin
+
get content of file
store = Store.find(store_id)
@@ -204,4 +275,32 @@ returns
file.provider
end
+
+ private
+
+ def image_resize(content, width)
+ local_sha = Digest::SHA256.hexdigest(content)
+
+ cache_key = "image-resize-#{local_sha}_#{width}"
+ all = nil
+ image = Cache.get(cache_key)
+ return image if image
+
+ temp_file = ::Tempfile.new
+ temp_file.binmode
+ temp_file.write(content)
+ temp_file.close
+ image = Rszr::Image.load(temp_file.path)
+ return if image.width < width
+
+ image.resize!(width, :auto)
+ temp_file_resize = ::Tempfile.new.path
+ image.save(temp_file_resize)
+ image_resized = ::File.binread(temp_file_resize)
+
+ Cache.write(cache_key, image_resized, { expires_in: 6.months })
+
+ image_resized
+ end
+
end
diff --git a/app/models/ticket.rb b/app/models/ticket.rb
index 740120b55..a5a81c1df 100644
--- a/app/models/ticket.rb
+++ b/app/models/ticket.rb
@@ -6,6 +6,7 @@ class Ticket < ApplicationModel
include ChecksClientNotification
include ChecksLatestChangeObserved
include CanCsvImport
+ include ChecksHtmlSanitized
include HasHistory
include HasTags
include HasSearchIndexBackend
@@ -56,6 +57,8 @@ class Ticket < ApplicationModel
history_relation_object 'Ticket::Article'
+ sanitized_html :note
+
belongs_to :group
belongs_to :organization
has_many :articles, class_name: 'Ticket::Article', after_add: :cache_update, after_remove: :cache_update, dependent: :destroy, inverse_of: :ticket
diff --git a/app/models/ticket/article.rb b/app/models/ticket/article.rb
index e5c1c56f7..d4f23bf83 100644
--- a/app/models/ticket/article.rb
+++ b/app/models/ticket/article.rb
@@ -82,7 +82,7 @@ returns
article['attachments'].each do |file|
next if !file[:preferences] || !file[:preferences]['Content-ID'] || (file[:preferences]['Content-ID'] != cid && file[:preferences]['Content-ID'] != "<#{cid}>" )
- replace = "#{tag_start}/api/v1/ticket_attachment/#{article['ticket_id']}/#{article['id']}/#{file[:id]}\"#{tag_end}>"
+ replace = "#{tag_start}/api/v1/ticket_attachment/#{article['ticket_id']}/#{article['id']}/#{file[:id]}?view=inline\"#{tag_end}>"
inline_attachments[file[:id]] = true
break
end
diff --git a/app/models/user.rb b/app/models/user.rb
index 5d564f088..280431476 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -6,6 +6,7 @@ class User < ApplicationModel
include HasHistory
include HasSearchIndexBackend
include CanCsvImport
+ include ChecksHtmlSanitized
include HasGroups
include HasRoles
@@ -66,6 +67,8 @@ class User < ApplicationModel
:groups,
:user_groups
+ sanitized_html :note
+
def ignore_search_indexing?(_action)
# ignore internal user
return true if id == 1
diff --git a/db/migrate/20190131000001_setting_change_ticket_zoom_attachment_preview.rb b/db/migrate/20190131000001_setting_change_ticket_zoom_attachment_preview.rb
new file mode 100644
index 000000000..296db7b3f
--- /dev/null
+++ b/db/migrate/20190131000001_setting_change_ticket_zoom_attachment_preview.rb
@@ -0,0 +1,34 @@
+class SettingChangeTicketZoomAttachmentPreview < ActiveRecord::Migration[5.1]
+ def up
+ # return if it's a new setup
+ return if !Setting.find_by(name: 'system_init_done')
+
+ Setting.create_or_update(
+ title: 'Sidebar Attachments',
+ name: 'ui_ticket_zoom_attachments_preview',
+ area: 'UI::TicketZoom::Preview',
+ description: 'Enables preview of attachments.',
+ options: {
+ form: [
+ {
+ display: '',
+ null: true,
+ name: 'ui_ticket_zoom_attachments_preview',
+ tag: 'boolean',
+ translate: true,
+ options: {
+ true => 'yes',
+ false => 'no',
+ },
+ },
+ ],
+ },
+ state: true,
+ preferences: {
+ prio: 400,
+ permission: ['admin.ui'],
+ },
+ frontend: true
+ )
+ end
+end
diff --git a/db/seeds/settings.rb b/db/seeds/settings.rb
index 778538f0c..0d6ce04e1 100644
--- a/db/seeds/settings.rb
+++ b/db/seeds/settings.rb
@@ -793,7 +793,7 @@ Setting.create_if_not_exists(
},
],
},
- state: false,
+ state: true,
preferences: {
prio: 400,
permission: ['admin.ui'],
diff --git a/lib/notification_factory/template.rb b/lib/notification_factory/template.rb
index 98ce92347..945eba14d 100644
--- a/lib/notification_factory/template.rb
+++ b/lib/notification_factory/template.rb
@@ -17,35 +17,31 @@ examples how to use
end
def to_s
- strip_html
- end
+ @template.gsub(/\#{\s*(.*?)\s*}/m) do
+ # some browsers start adding HTML tags
+ # fixes https://github.com/zammad/zammad/issues/385
+ input_template = $1.gsub(/\A<.+?>\s*|\s*<.+?>\z/, '')
- def strip_html
- # some browsers start adding HTML tags
- # fixes https://github.com/zammad/zammad/issues/385
- @template.gsub(/\#\{\s*t\((.+?)\)\s*\}/m) do
- content = $1
- if content =~ /^'(.+?)'$/
- %(<%= t "#{strip_content($1)}", #{@escape} %>)
+ case input_template
+ when /\At\('(.+?)'\)\z/m
+ %(<%= t "#{sanitize_text($1)}", #{@escape} %>)
+ when /\At\((.+?)\)\z/m
+ %(<%= t d"#{sanitize_object_name($1)}", #{@escape} %>)
+ when /\Aconfig\.(.+?)\z/m
+ %(<%= c "#{sanitize_object_name($1)}", #{@escape} %>)
else
- %(<%= t d"#{strip_variable(content)}", #{@escape} %>)
+ %(<%= d "#{sanitize_object_name(input_template)}", #{@escape} %>)
end
- end.gsub(/\#\{\s*config\.(.+?)\s*\}/m) do
- %(<%= c "#{strip_variable($1)}", #{@escape} %>)
- end.gsub(/\#\{(.*?)\}/m) do
- %(<%= d "#{strip_variable($1)}", #{@escape} %>)
end
end
- def strip_content(string)
- string&.gsub(/\t|\r|\n/, '')
- &.gsub(/"/, '\"')
+ def sanitize_text(string)
+ string&.tr("\t\r\n", '')
+ &.gsub(/(?/, '')
+ def sanitize_object_name(string)
+ string&.tr("\t\r\n\f \"'§;", '')
end
end
diff --git a/public/assets/chat/chat-no-jquery.coffee b/public/assets/chat/chat-no-jquery.coffee
index f43d1285b..7b8fe5cc7 100644
--- a/public/assets/chat/chat-no-jquery.coffee
+++ b/public/assets/chat/chat-no-jquery.coffee
@@ -501,7 +501,7 @@ do(window) ->
stopPropagation: (event) ->
event.stopPropagation()
- onPaste: (e) =>
+ onDrop: (e) =>
e.stopPropagation()
e.preventDefault()
diff --git a/spec/lib/notification_factory/template_spec.rb b/spec/lib/notification_factory/template_spec.rb
new file mode 100644
index 000000000..094f5ca46
--- /dev/null
+++ b/spec/lib/notification_factory/template_spec.rb
@@ -0,0 +1,91 @@
+require 'rails_helper'
+
+RSpec.describe NotificationFactory::Template do
+ subject(:template) do
+ NotificationFactory::Template.new(template_string, escape)
+ end
+
+ describe '#to_s' do
+ context 'for empty input template (incl. whitespace-only)' do
+ let(:template_string) { "\#{ }" }
+
+ context 'with escape = true' do
+ let(:escape) { true }
+
+ it 'returns an ERB template with the #d helper, and passes escape arg as string' do
+ expect(template.to_s).to eq('<%= d "", true %>')
+ end
+ end
+
+ context 'with escape = false' do
+ let(:escape) { false }
+
+ it 'returns an ERB template with the #d helper, and passes escape arg as string' do
+ expect(template.to_s).to eq('<%= d "", false %>')
+ end
+ end
+ end
+
+ context 'for input template using #t helper' do
+ let(:template_string) { "\#{t('some text')}" }
+ let(:escape) { false }
+
+ it 'returns an ERB template with the #t helper, and passes escape arg as string' do
+ expect(template.to_s).to eq('<%= t "some text", false %>')
+ end
+
+ context 'with double-quotes in argument' do
+ let(:template_string) { "\#{t('some \"text\"')}" }
+
+ it 'adds backslash-escaping' do
+ expect(template.to_s).to eq('<%= t "some \"text\"", false %>')
+ end
+ end
+ end
+
+ # Regression test for https://github.com/zammad/zammad/issues/385
+ context 'with HTML auto-injected by browser' do
+ let(:escape) { true }
+
+ context 'for
tags wrapped around "ticket.id"' do
+ let(:template_string) { <<~'TEMPLATE'.chomp }
+ #{ticket.id}
+ TEMPLATE
+
+ it 'strips tag from resulting ERB template' do
+ expect(template.to_s).to eq('<%= d "ticket.id", true %>')
+ end
+ end
+
+ context 'for
tags wrapped around "config.fqdn"' do
+ let(:template_string) { <<~'TEMPLATE'.chomp }
+ #{config.fqdn}
+ TEMPLATE
+
+ it 'strips tag from resulting ERB template' do
+ expect(template.to_s).to eq('<%= c "fqdn", true %>')
+ end
+ end
+
+ context 'for
tags surrounded by whitespace' do
+ let(:template_string) { <<~'TEMPLATE'.chomp }
+ #{ ticket.id }
+ TEMPLATE
+
+ it 'strips tag and spaces from template' do
+ expect(template.to_s).to eq('<%= d "ticket.id", true %>')
+ end
+ end
+
+ context 'for unpaired
tag and trailing whitespace' do
+ let(:template_string) { <<~'TEMPLATE'.chomp }
+ #{ticket.id }
+ TEMPLATE
+
+ it 'strips tag and spaces from template' do
+ expect(template.to_s).to eq('<%= d "ticket.id", true %>')
+ end
+ end
+ end
+ end
+end
diff --git a/spec/models/concerns/has_xss_sanitized_note_examples.rb b/spec/models/concerns/has_xss_sanitized_note_examples.rb
new file mode 100644
index 000000000..8af57b840
--- /dev/null
+++ b/spec/models/concerns/has_xss_sanitized_note_examples.rb
@@ -0,0 +1,10 @@
+RSpec.shared_examples 'HasXssSanitizedNote' do |model_factory:|
+ describe 'XSS prevention' do
+ context 'with injected JS' do
+ subject { create(model_factory, note: 'test 123 some text') }
+ it 'strips out