From 80f669d8eb7b8c348fd0b9633d18d9703d236912 Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Fri, 8 Sep 2017 09:24:07 +0200 Subject: [PATCH] Fixed issue #1390 - Broken email with huge pdf inline (lead to huge article body with bigger then 7MB) is blocking rendering of web app. --- app/models/ticket/article.rb | 20 +++++- test/unit/ticket_article_dos_test.rb | 103 +++++++++++++++++++++++++++ 2 files changed, 121 insertions(+), 2 deletions(-) create mode 100644 test/unit/ticket_article_dos_test.rb diff --git a/app/models/ticket/article.rb b/app/models/ticket/article.rb index 8bc557bfe..372e83ff9 100644 --- a/app/models/ticket/article.rb +++ b/app/models/ticket/article.rb @@ -16,8 +16,8 @@ class Ticket::Article < ApplicationModel belongs_to :created_by, class_name: 'User' belongs_to :updated_by, class_name: 'User' store :preferences - before_create :check_subject, :check_message_id_md5 - before_update :check_subject, :check_message_id_md5 + before_create :check_subject, :check_body, :check_message_id_md5 + before_update :check_subject, :check_body, :check_message_id_md5 sanitized_html :body @@ -284,6 +284,22 @@ returns true end + # strip body length or raise exception + def check_body + return true if body.blank? + limit = 1_500_000 + current_length = body.length + if body.length > limit + if ApplicationHandleInfo.current.present? && ApplicationHandleInfo.current.split('.')[1] == 'postmaster' + logger.warn "WARNING: cut string because of database length #{self.class}.body(#{limit} but is #{current_length})" + self.body = body[0, limit] + else + raise Exceptions::UnprocessableEntity, "body if article is to large, #{current_length} chars - only #{limit} allowed" + end + end + true + end + def history_log_attributes { related_o_id: self['ticket_id'], diff --git a/test/unit/ticket_article_dos_test.rb b/test/unit/ticket_article_dos_test.rb new file mode 100644 index 000000000..cd75dd747 --- /dev/null +++ b/test/unit/ticket_article_dos_test.rb @@ -0,0 +1,103 @@ +# encoding: utf-8 +require 'test_helper' + +class TicketArticleDos < ActiveSupport::TestCase + + test 'check body size' do + + org_community = Organization.create_if_not_exists( + name: 'Zammad Foundation', + ) + user_community = User.create_or_update( + login: 'article.dos@example.org', + firstname: 'Article', + lastname: 'Dos', + email: 'article.dos@example.org', + password: '', + active: true, + roles: [ Role.find_by(name: 'Customer') ], + organization_id: org_community.id, + updated_by_id: 1, + created_by_id: 1, + ) + + UserInfo.current_user_id = user_community.id + ApplicationHandleInfo.current = 'test.postmaster' + + ticket1 = Ticket.create!( + group_id: Group.first.id, + customer_id: user_community.id, + title: 'DoS 1!', + updated_by_id: 1, + created_by_id: 1, + ) + article1 = Ticket::Article.create!( + ticket_id: ticket1.id, + type_id: Ticket::Article::Type.find_by(name: 'phone').id, + sender_id: Ticket::Article::Sender.find_by(name: 'Customer').id, + from: 'Zammad Feedback ', + body: Array.new(2_000_000) { [*'0'..'9', *'a'..'z', ' ', ' ', ' ', '. '].sample }.join, + internal: false, + updated_by_id: 1, + created_by_id: 1, + ) + assert_equal(1_500_000, article1.body.length) + + ticket2 = Ticket.create!( + group_id: Group.first.id, + customer_id: user_community.id, + title: 'DoS 2!', + updated_by_id: 1, + created_by_id: 1, + ) + article2 = Ticket::Article.create!( + ticket_id: ticket2.id, + type_id: Ticket::Article::Type.find_by(name: 'phone').id, + sender_id: Ticket::Article::Sender.find_by(name: 'Customer').id, + from: 'Zammad Feedback ', + body: "\u0000#{Array.new(2_000_000) { [*'0'..'9', *'a'..'z', ' ', ' ', ' ', '. '].sample }.join}", + internal: false, + updated_by_id: 1, + created_by_id: 1, + ) + assert_equal(1_500_000, article2.body.length) + + ApplicationHandleInfo.current = 'web' + + ticket3 = Ticket.create!( + group_id: Group.first.id, + customer_id: user_community.id, + title: 'DoS 3!', + updated_by_id: 1, + created_by_id: 1, + ) + + assert_raises(Exceptions::UnprocessableEntity) { + article3 = Ticket::Article.create!( + ticket_id: ticket3.id, + type_id: Ticket::Article::Type.find_by(name: 'phone').id, + sender_id: Ticket::Article::Sender.find_by(name: 'Customer').id, + from: 'Zammad Feedback ', + body: "\u0000#{Array.new(2_000_000) { [*'0'..'9', *'a'..'z', ' ', ' ', ' ', '. '].sample }.join}", + internal: false, + updated_by_id: 1, + created_by_id: 1, + ) + } + + end + + test 'check body size / cut if email' do + + email_raw_string = "From: me@example.com +To: customer@example.com +Subject: some new subject + +Some Text" + Array.new(2_000_000) { [*'0'..'9', *'a'..'z', ' ', ' ', ' ', '. '].sample }.join + + ticket_p, article_p, user_p, mail = Channel::EmailParser.new.process({}, email_raw_string) + assert_equal(1_500_000, article_p.body.length) + + end + +end