CVE-2020-14213: Missing Authorization (CWE-862)
value: In Zammad before 3.3.1, a Customer has ticket access that should only be available to an Agent (e.g., read internal data, split, or merge).
CWE description: The product does not perform an authorization check when an actor attempts to access a resource or perform an action.
CVSS v3 base score: 5.4
Published: 2020-06-16T23:15Z
Repository: https://github.com/zammad/zammad
Language: Ruby
Files changed: app/controllers/tickets_controller.rb, app/policies/controllers/tickets_controller_policy.rb, spec/requests/ticket_spec.rb
Commit date: 2020-06-10 13:39:54 +0200
Commit message: Enhancement: Added authorization checks for ticket controller actions: ticket_customer ticket_history ticket_related ticket_recent ticket_merge ticket_split.

Diff: diff --git a/app/controllers/tickets_controller.rb b/app/controllers/tickets_controller.rb
index d7e02f84b..60edd6fc9 100644
--- a/app/controllers/tickets_controller.rb
+++ b/app/controllers/tickets_controller.rb
@@ -4,11 +4,11 @@ class TicketsController < ApplicationController
   include CreatesTicketArticles
   include ClonesTicketArticleAttachments
   include ChecksUserAttributesByCurrentUserPermission
   include TicketStats
 
-  prepend_before_action -> { authorize! }, only: %i[create selector import_example import_start]
+  prepend_before_action -> { authorize! }, only: %i[create selector import_example import_start ticket_customer ticket_history ticket_related ticket_recent ticket_merge ticket_split]
   prepend_before_action :authentication_check
 
   # GET /api/v1/tickets
   def index
     offset = 0
diff --git a/app/policies/controllers/tickets_controller_policy.rb b/app/policies/controllers/tickets_controller_policy.rb
index 2aadda786..b7d5ca676 100644
--- a/app/policies/controllers/tickets_controller_policy.rb
+++ b/app/policies/controllers/tickets_controller_policy.rb
@@ -1,5 +1,6 @@
 class Controllers::TicketsControllerPolicy < Controllers::ApplicationControllerPolicy
   permit! %i[import_example import_start], to: 'admin'
   permit! :selector, to: 'admin.*'
+  permit! %i[ticket_customer ticket_history ticket_related ticket_recent ticket_merge ticket_split], to: 'ticket.agent'
   permit! :create, to: ['ticket.agent', 'ticket.customer']
 end
diff --git a/spec/requests/ticket_spec.rb b/spec/requests/ticket_spec.rb
index e2cc32b51..7e7169675 100644
--- a/spec/requests/ticket_spec.rb
+++ b/spec/requests/ticket_spec.rb
@@ -1791,10 +1791,14 @@ RSpec.describe 'Ticket', type: :request do
           'Content-Disposition' => 'attachment',
         },
         created_by_id: 1,
       )
 
+      authenticated_as(customer_user)
+      get "/api/v1/ticket_split?ticket_id=#{ticket.id}&article_id=#{article.id}&form_id=new_form_id123", params: {}, as: :json
+      expect(response).to have_http_status(:unauthorized)
+
       authenticated_as(agent_user)
       get "/api/v1/ticket_split?ticket_id=#{ticket.id}&article_id=#{article.id}&form_id=new_form_id123", params: {}, as: :json
       expect(response).to have_http_status(:ok)
       expect(json_response).to be_a_kind_of(Hash)
       expect(json_response['assets']).to be_truthy
@@ -1916,10 +1920,14 @@ RSpec.describe 'Ticket', type: :request do
         title:       'ticket merge2',
         group:       group_no_permission,
         customer_id: customer_user.id,
       )
 
+      authenticated_as(customer_user)
+      get "/api/v1/ticket_merge/#{ticket2.id}/#{ticket1.id}", params: {}, as: :json
+      expect(response).to have_http_status(:unauthorized)
+
       authenticated_as(agent_user)
       get "/api/v1/ticket_merge/#{ticket2.id}/#{ticket1.id}", params: {}, as: :json
       expect(response).to have_http_status(:ok)
       expect(json_response).to be_a_kind_of(Hash)
       expect(json_response['result']).to eq('failed')
@@ -2066,11 +2074,43 @@ RSpec.describe 'Ticket', type: :request do
       expect(json_response).to be_a_kind_of(Hash)
       expect(json_response['history'].class).to eq(Array)
       expect(json_response['assets'].class).to eq(Hash)
       expect(json_response['assets']['User'][customer_user.id.to_s]).not_to be_nil
       expect(json_response['assets']['Ticket'][ticket1.id.to_s]).not_to be_nil
+
+      authenticated_as(customer_user)
+      get "/api/v1/ticket_history/#{ticket1.id}", params: {}, as: :json
+      expect(response).to have_http_status(:unauthorized)
+    end
+
+    it 'does ticket related' do
+      ticket1 = create(
+        :ticket,
+        title:       'some title',
+        group:       ticket_group,
+        customer_id: customer_user.id,
+      )
+
+      authenticated_as(agent_user)
+      get "/api/v1/ticket_related/#{ticket1.id}", params: {}, as: :json
+      expect(response).to have_http_status(:ok)
+
+      authenticated_as(customer_user)
+      get "/api/v1/ticket_related/#{ticket1.id}", params: {}, as: :json
+      expect(response).to have_http_status(:unauthorized)
+    end
+
+    it 'does ticket recent' do
+      authenticated_as(agent_user)
+      get '/api/v1/ticket_recent', params: {}, as: :json
+      expect(response).to have_http_status(:ok)
+
+      authenticated_as(customer_user)
+      get '/api/v1/ticket_recent', params: {}, as: :json
+      expect(response).to have_http_status(:unauthorized)
     end
+
   end
 
   describe 'stats' do
     let(:ticket1) { create(:ticket, customer: customer, organization: organization, group: ticket_group) }
     let(:ticket2) { create(:ticket, customer: customer, organization: organization, group: ticket_group) }
@@ -2211,11 +2251,11 @@ RSpec.describe 'Ticket', type: :request do
       context 'as agent', authenticated_as: -> { agent } do
         include_examples 'has access'
       end
 
       context 'as authorized customer', authenticated_as: -> { customer_authorized } do
-        include_examples 'has access'
+        include_examples 'has no access'
       end
 
       context 'as unauthorized customer', authenticated_as: -> { customer_unauthorized } do
         include_examples 'has no access'
       end

Vulnerable code:   prepend_before_action -> { authorize! }, only: %i[create selector import_example import_start]
        include_examples 'has access'
Fixed code:   prepend_before_action -> { authorize! }, only: %i[create selector import_example import_start ticket_customer ticket_history ticket_related ticket_recent ticket_merge ticket_split]
  permit! %i[ticket_customer ticket_history ticket_related ticket_recent ticket_merge ticket_split], to: 'ticket.agent'
      authenticated_as(customer_user)
      get "/api/v1/ticket_split?ticket_id=#{ticket.id}&article_id=#{article.id}&form_id=new_form_id123", params: {}, as: :json
      expect(response).to have_http_status(:unauthorized)

      authenticated_as(customer_user)
      get "/api/v1/ticket_merge/#{ticket2.id}/#{ticket1.id}", params: {}, as: :json
      expect(response).to have_http_status(:unauthorized)


      authenticated_as(customer_user)
      get "/api/v1/ticket_history/#{ticket1.id}", params: {}, as: :json
      expect(response).to have_http_status(:unauthorized)
    end

    it 'does ticket related' do
      ticket1 = create(
        :ticket,
        title:       'some title',
        group:       ticket_group,
        customer_id: customer_user.id,
      )

      authenticated_as(agent_user)
      get "/api/v1/ticket_related/#{ticket1.id}", params: {}, as: :json
      expect(response).to have_http_status(:ok)

      authenticated_as(customer_user)
      get "/api/v1/ticket_related/#{ticket1.id}", params: {}, as: :json
      expect(response).to have_http_status(:unauthorized)
    end

    it 'does ticket recent' do
      authenticated_as(agent_user)
      get '/api/v1/ticket_recent', params: {}, as: :json
      expect(response).to have_http_status(:ok)

      authenticated_as(customer_user)
      get '/api/v1/ticket_recent', params: {}, as: :json
      expect(response).to have_http_status(:unauthorized)

        include_examples 'has no access'