From 4479e180bd74e2618a7aa4702f747463d70d0407 Mon Sep 17 00:00:00 2001
From: Billy Zhou <bz@zammad.com>
Date: Wed, 25 Jul 2018 14:26:59 +0800
Subject: [PATCH] Fixed issue #2141 by switching to strict_decode64 for
 decoding base64

---
 .../concerns/creates_ticket_articles.rb       |  7 +-
 test/controllers/tickets_controller_test.rb   | 79 ++++++++++++++++++-
 2 files changed, 83 insertions(+), 3 deletions(-)

diff --git a/app/controllers/concerns/creates_ticket_articles.rb b/app/controllers/concerns/creates_ticket_articles.rb
index a048cbe81..df7741c3a 100644
--- a/app/controllers/concerns/creates_ticket_articles.rb
+++ b/app/controllers/concerns/creates_ticket_articles.rb
@@ -94,14 +94,17 @@ module CreatesTicketArticles
           preferences[store_key] = attachment[key]
         end
 
-        if !attachment[:data].match?(%r{^([A-Za-z0-9+/]{4})*([A-Za-z0-9+/]{4}|[A-Za-z0-9+/]{3}=|[A-Za-z0-9+/]{2}==)$})
+        begin
+          base64_data = attachment[:data].gsub(/[\r\n]/, '')
+          attachment_data = Base64.strict_decode64(base64_data)
+        rescue ArgumentError => e
           raise Exceptions::UnprocessableEntity, "Invalid base64 for attachment with index '#{index}'"
         end
 
         Store.add(
           object: 'Ticket::Article',
           o_id: article.id,
-          data: Base64.decode64(attachment[:data]),
+          data: attachment_data,
           filename: attachment[:filename],
           preferences: preferences,
         )
diff --git a/test/controllers/tickets_controller_test.rb b/test/controllers/tickets_controller_test.rb
index 3e2f59689..637969447 100644
--- a/test/controllers/tickets_controller_test.rb
+++ b/test/controllers/tickets_controller_test.rb
@@ -551,7 +551,7 @@ class TicketsControllerTest < ActionDispatch::IntegrationTest
     assert_not(file.preferences['Content-ID'])
   end
 
-  test '01.15 ticket create with agent - minimal article and attachment missing mine-type with customer' do
+  test '01.15 ticket create with agent - minimal article and simple invalid base64 attachment with customer' do
     credentials = ActionController::HttpAuthentication::Basic.encode_credentials('tickets-agent@example.com', 'agentpw')
     params = {
       title: 'a new ticket #15',
@@ -574,6 +574,83 @@ class TicketsControllerTest < ActionDispatch::IntegrationTest
     assert_equal('Invalid base64 for attachment with index \'0\'', result['error'])
   end
 
+  test '01.15a ticket create with agent - minimal article and large invalid base64 attachment with customer' do
+    credentials = ActionController::HttpAuthentication::Basic.encode_credentials('tickets-agent@example.com', 'agentpw')
+    params = {
+      title: 'a new ticket #15a',
+      group: 'Users',
+      customer_id: @customer_without_org.id,
+      article: {
+        subject: 'some test 123',
+        body: 'some test 123',
+        attachments: [
+          'filename' => 'some_file.txt',
+          'data' => "LARGE_INVALID_BASE64_#{'#' * 20_000_000}",
+          'mime-type' => 'text/plain',
+        ],
+      },
+    }
+    post '/api/v1/tickets', params: params.to_json, headers: @headers.merge('Authorization' => credentials)
+    assert_response(422)
+    result = JSON.parse(@response.body)
+    assert_equal(Hash, result.class)
+    assert_equal('Invalid base64 for attachment with index \'0\'', result['error'])
+  end
+
+  test '01.15b ticket create with agent - minimal article and valid multiline base64 with linebreaks attachment with customer' do
+    credentials = ActionController::HttpAuthentication::Basic.encode_credentials('tickets-agent@example.com', 'agentpw')
+    params = {
+      title: 'a new ticket #15b',
+      group: 'Users',
+      customer_id: @customer_without_org.id,
+      article: {
+        subject: 'some test 123',
+        body: 'some test 123',
+        attachments: [
+          'filename' => 'some_file.txt',
+          'data' =>  Base64.encode64('a' * 1_000),
+          'mime-type' => 'text/plain',
+        ],
+      },
+    }
+    post '/api/v1/tickets', params: params.to_json, headers: @headers.merge('Authorization' => credentials)
+    assert_response(201)
+    result = JSON.parse(@response.body)
+    assert_equal('a new ticket #15b', result['title'])
+    ticket = Ticket.find(result['id'])
+    assert_equal(1, ticket.articles.count)
+    assert_equal(1, ticket.articles.first.attachments.count)
+    file = ticket.articles.first.attachments.first
+    assert_equal('a' * 1_000, file.content)
+  end
+
+  test '01.15c ticket create with agent - minimal article and valid multiline base64 without linebreaks attachment with customer' do
+    credentials = ActionController::HttpAuthentication::Basic.encode_credentials('tickets-agent@example.com', 'agentpw')
+    params = {
+      title: 'a new ticket #15c',
+      group: 'Users',
+      customer_id: @customer_without_org.id,
+      article: {
+        subject: 'some test 123',
+        body: 'some test 123',
+        attachments: [
+          'filename' => 'some_file.txt',
+          'data' =>  Base64.strict_encode64('a' * 1_000),
+          'mime-type' => 'text/plain',
+        ],
+      },
+    }
+    post '/api/v1/tickets', params: params.to_json, headers: @headers.merge('Authorization' => credentials)
+    assert_response(201)
+    result = JSON.parse(@response.body)
+    assert_equal('a new ticket #15c', result['title'])
+    ticket = Ticket.find(result['id'])
+    assert_equal(1, ticket.articles.count)
+    assert_equal(1, ticket.articles.first.attachments.count)
+    file = ticket.articles.first.attachments.first
+    assert_equal('a' * 1_000, file.content)
+  end
+
   test '01.16 ticket create with agent - minimal article and attachment invalid base64 with customer' do
     credentials = ActionController::HttpAuthentication::Basic.encode_credentials('tickets-agent@example.com', 'agentpw')
     params = {