Added HTML Sanitzer concern for multi model usage.

This commit is contained in:
Thorsten Eckel 2017-02-01 12:48:50 +01:00
parent ef6fbe8f2a
commit 8800a4a861
6 changed files with 293 additions and 1 deletions

View file

@ -1,2 +1,5 @@
class Chat::Message < ApplicationModel class Chat::Message < ApplicationModel
include HtmlSanitized
sanitized_html :content
end end

View file

@ -0,0 +1,46 @@
# Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/
module HtmlSanitized
extend ActiveSupport::Concern
included do
before_create :sanitized_html_attributes
before_update :sanitized_html_attributes
end
def sanitized_html_attributes
html_attributes = self.class.instance_variable_get(:@sanitized_html) || []
return if html_attributes.empty?
html_attributes.each do |attribute|
value = send(attribute)
next if value.blank?
next if !sanitizeable?(attribute, value)
send("#{attribute}=".to_sym, HtmlSanitizer.strict(value))
end
end
def sanitizeable?(_attribute, _value)
true
end
# methods defined here are going to extend the class, not the instance of it
class_methods do
=begin
serve methode to mark HTML attrbibutes that need to get sanitized
class Model < ApplicationModel
include Sanitized
sanitized_html :body
end
=end
def sanitized_html(*attributes)
@sanitized_html = attributes
end
end
end

View file

@ -3,6 +3,7 @@ class Ticket::Article < ApplicationModel
include LogsActivityStream include LogsActivityStream
include NotifiesClients include NotifiesClients
include Historisable include Historisable
include HtmlSanitized
load 'ticket/article/assets.rb' load 'ticket/article/assets.rb'
include Ticket::Article::Assets include Ticket::Article::Assets
@ -16,6 +17,8 @@ class Ticket::Article < ApplicationModel
before_create :check_subject, :check_message_id_md5 before_create :check_subject, :check_message_id_md5
before_update :check_subject, :check_message_id_md5 before_update :check_subject, :check_message_id_md5
sanitized_html :body
activity_stream_permission 'ticket.agent' activity_stream_permission 'ticket.agent'
activity_stream_attributes_ignored :type_id, activity_stream_attributes_ignored :type_id,
@ -203,6 +206,12 @@ returns:
) )
end end
def sanitizeable?(attribute, _value)
return true if attribute != :body
return false if content_type.blank?
content_type =~ /html/i
end
private private
# strip not wanted chars # strip not wanted chars

48
lib/html_sanitizer.rb Normal file
View file

@ -0,0 +1,48 @@
class HtmlSanitizer
def self.strict(string)
remove = %w(style body head)
strip = ['script']
scrubber = Loofah::Scrubber.new do |node|
# strip tags
if strip.include?(node.name)
node.before node.children
node.remove
end
# remove tags
if remove.include?(node.name)
node.remove
end
# prepare src attribute
if node['src']
if node['src'].downcase.start_with?('http', 'ftp')
node.before node.children
node.remove
end
end
# prepare links
if node['href']
if node['href'].downcase.start_with?('http', 'ftp')
node.set_attribute('rel', 'nofollow')
node.set_attribute('target', '_blank')
end
if node['href'] =~ /javascript/i
node.delete('href')
end
end
# remove on* attributes
node.each { |attribute, _value|
next if !attribute.downcase.start_with?('on')
node.delete(attribute)
}
end
Loofah.fragment(string).scrub!(scrubber).to_s
end
end

File diff suppressed because one or more lines are too long

View file

@ -0,0 +1,186 @@
# encoding: utf-8
require 'test_helper'
class TicketXssTest < ActiveSupport::TestCase
test 'xss via model' do
ticket = Ticket.create(
title: 'test 123 <script type="text/javascript">alert("XSS!");</script>',
group: Group.lookup(name: 'Users'),
customer_id: 2,
state: Ticket::State.lookup(name: 'new'),
priority: Ticket::Priority.lookup(name: '2 normal'),
updated_by_id: 1,
created_by_id: 1,
)
assert(ticket, 'ticket created')
assert_equal('test 123 <script type="text/javascript">alert("XSS!");</script>', ticket.title, 'ticket.title verify')
assert_equal('Users', ticket.group.name, 'ticket.group verify')
assert_equal('new', ticket.state.name, 'ticket.state verify')
article1 = Ticket::Article.create(
ticket_id: ticket.id,
from: 'some_sender@example.com',
to: 'some_recipient@example.com',
subject: 'some subject <script type="text/javascript">alert("XSS!");</script>',
message_id: 'some@id',
content_type: 'text/html',
body: '<script type="text/javascript">alert("XSS!");</script>',
internal: false,
sender: Ticket::Article::Sender.find_by(name: 'Customer'),
type: Ticket::Article::Type.find_by(name: 'email'),
updated_by_id: 1,
created_by_id: 1,
)
assert_equal('alert("XSS!");', article1.body, 'article1.body verify - inbound')
article2 = Ticket::Article.create(
ticket_id: ticket.id,
from: 'some_sender@example.com',
to: 'some_recipient@example.com',
subject: 'some subject <script type="text/javascript">alert("XSS!");</script>',
message_id: 'some@id',
content_type: 'text/html',
body: 'please tell me this doesn\'t work: <script type="text/javascript">alert("XSS!");</script>',
internal: false,
sender: Ticket::Article::Sender.find_by(name: 'Customer'),
type: Ticket::Article::Type.find_by(name: 'email'),
updated_by_id: 1,
created_by_id: 1,
)
assert_equal('please tell me this doesn\'t work: alert("XSS!");', article2.body, 'article2.body verify - inbound')
article3 = Ticket::Article.create(
ticket_id: ticket.id,
from: 'some_sender@example.com',
to: 'some_recipient@example.com',
subject: 'some subject <script type="text/javascript">alert("XSS!");</script>',
message_id: 'some@id',
content_type: 'text/html',
body: 'please tell me this doesn\'t work: <table>ada<tr></tr></table><div class="adasd" id="123" data-abc="123"></div><div><a href="javascript:someFunction()">LINK</a><a href="http://lalal.de">aa</a><some_not_existing>ABC</some_not_existing>',
internal: false,
sender: Ticket::Article::Sender.find_by(name: 'Customer'),
type: Ticket::Article::Type.find_by(name: 'email'),
updated_by_id: 1,
created_by_id: 1,
)
assert_equal("please tell me this doesn't work: <table>ada<tr></tr>
</table><div class=\"adasd\" id=\"123\" data-abc=\"123\"></div><div>
<a>LINK</a><a href=\"http://lalal.de\" rel=\"nofollow\" target=\"_blank\">aa</a><some_not_existing>ABC</some_not_existing>
</div>", article3.body, 'article3.body verify - inbound')
article4 = Ticket::Article.create(
ticket_id: ticket.id,
from: 'some_sender@example.com',
to: 'some_recipient@example.com',
subject: 'some subject <script type="text/javascript">alert("XSS!");</script>',
message_id: 'some@id',
content_type: 'text/html',
body: 'please tell me this doesn\'t work: <video>some video</video><foo>alal</foo>',
internal: false,
sender: Ticket::Article::Sender.find_by(name: 'Customer'),
type: Ticket::Article::Type.find_by(name: 'email'),
updated_by_id: 1,
created_by_id: 1,
)
assert_equal("please tell me this doesn't work: <video>some video</video><foo>alal</foo>", article4.body, 'article4.body verify - inbound')
article5 = Ticket::Article.create(
ticket_id: ticket.id,
from: 'some_sender@example.com',
to: 'some_recipient@example.com',
subject: 'some subject <script type="text/javascript">alert("XSS!");</script>',
message_id: 'some@id',
content_type: 'text/plain',
body: 'please tell me this doesn\'t work: <table>ada<tr></tr></table><div class="adasd" id="123" data-abc="123"></div><div><a href="javascript:someFunction()">LINK</a><a href="http://lalal.de">aa</a><some_not_existing>ABC</some_not_existing>',
internal: false,
sender: Ticket::Article::Sender.find_by(name: 'Customer'),
type: Ticket::Article::Type.find_by(name: 'email'),
updated_by_id: 1,
created_by_id: 1,
)
assert_equal('please tell me this doesn\'t work: <table>ada<tr></tr></table><div class="adasd" id="123" data-abc="123"></div><div><a href="javascript:someFunction()">LINK</a><a href="http://lalal.de">aa</a><some_not_existing>ABC</some_not_existing>', article5.body, 'article5.body verify - inbound')
article6 = Ticket::Article.create(
ticket_id: ticket.id,
from: 'some_sender@example.com',
to: 'some_recipient@example.com',
subject: 'some subject <script type="text/javascript">alert("XSS!");</script>',
message_id: 'some@id',
content_type: 'text/html',
body: 'some message article helper test1 <div><img style="width: 85.5px; height: 49.5px" src="cid:15.274327094.140938@zammad.example.com">asdasd<img src="cid:15.274327094.140939@zammad.example.com"><br>',
internal: false,
sender: Ticket::Article::Sender.find_by(name: 'Customer'),
type: Ticket::Article::Type.find_by(name: 'email'),
updated_by_id: 1,
created_by_id: 1,
)
assert_equal('some message article helper test1 <div>
<img style="width: 85.5px; height: 49.5px" src="cid:15.274327094.140938@zammad.example.com">asdasd<img src="cid:15.274327094.140939@zammad.example.com"><br>
</div>', article6.body, 'article6.body verify - inbound')
article7 = Ticket::Article.create(
ticket_id: ticket.id,
from: 'some_sender@example.com',
to: 'some_recipient@example.com',
subject: 'some subject <script type="text/javascript">alert("XSS!");</script>',
message_id: 'some@id',
content_type: 'text/html',
body: 'some message article helper test1 <div><img style="width: 85.5px; height: 49.5px" src="api/v1/ticket_attachment/123/123/123">asdasd<img src="api/v1/ticket_attachment/123/123/123"><br>',
internal: false,
sender: Ticket::Article::Sender.find_by(name: 'Customer'),
type: Ticket::Article::Type.find_by(name: 'email'),
updated_by_id: 1,
created_by_id: 1,
)
assert_equal('some message article helper test1 <div>
<img style="width: 85.5px; height: 49.5px" src="api/v1/ticket_attachment/123/123/123">asdasd<img src="api/v1/ticket_attachment/123/123/123"><br>
</div>', article7.body, 'article7.body verify - inbound')
article8 = Ticket::Article.create(
ticket_id: ticket.id,
from: 'some_sender@example.com',
to: 'some_recipient@example.com',
subject: 'some subject <script type="text/javascript">alert("XSS!");</script>',
message_id: 'some@id',
content_type: 'text/html',
body: 'some message article helper test1 <a href="#" onclick="some_function();">abc</a> <a href="https://example.com" oNclIck="some_function();">123</a><body>123</body>',
internal: false,
sender: Ticket::Article::Sender.find_by(name: 'Customer'),
type: Ticket::Article::Type.find_by(name: 'email'),
updated_by_id: 1,
created_by_id: 1,
)
assert_equal('some message article helper test1 <a href="#">abc</a> <a href="https://example.com" rel="nofollow" target="_blank">123</a>123', article8.body, 'article8.body verify - inbound')
end
test 'xss via mail' do
data = 'From: ME Bob <me@example.com>
To: customer@example.com
Subject: some subject
Content-Type: text/html
MIME-Version: 1.0
no HTML <script type="text/javascript">alert(\'XSS\')</script>'
parser = Channel::EmailParser.new
ticket, article, user = parser.process({}, data)
assert_equal('text/html', ticket.articles.first.content_type)
assert_equal('no HTML alert(\'XSS\')', ticket.articles.first.body)
data = 'From: ME Bob <me@example.com>
To: customer@example.com
Subject: some subject
Content-Type: text/plain
MIME-Version: 1.0
no HTML <script type="text/javascript">alert(\'XSS\')</script>'
parser = Channel::EmailParser.new
ticket, article, user = parser.process({}, data)
assert_equal('text/plain', ticket.articles.first.content_type)
assert_equal('no HTML <script type="text/javascript">alert(\'XSS\')</script>', ticket.articles.first.body)
end
end