CVE-2017-2662

@@ -5,6 +5,7 @@ class Api::V2::ApiController < ::Api::V2::BaseController
     include Api::V2::Rendering
     include Api::V2::ErrorHandling
     include ::Foreman::Controller::CsvResponder
+    include Concerns::Api::V2::AssociationsPermissionCheck
 
     # support for session (thread-local) variables must be the last filter in this class
     include Foreman::ThreadSession::Cleaner

@@ -22,6 +22,13 @@ class Api::V2::ContentViewsController < Api::V2::ApiController
       param :solve_dependencies, :bool, :desc => N_("Solve RPM dependencies by default on Content View publish, defaults to false")
     end
 
+    def filtered_associations
+      {
+        :component_ids => Katello::ContentViewVersion,
+        :repository_ids => Katello::Repository
+      }
+    end
+
     api :GET, "/organizations/:organization_id/content_views", N_("List content views")
     api :GET, "/content_views", N_("List content views")
     param :organization_id, :number, :desc => N_("organization identifier")

@@ -0,0 +1,67 @@
+module Katello
+  module Concerns
+    module Api::V2::AssociationsPermissionCheck
+      extend ActiveSupport::Concern
+
+      # The purpose of this module is to protect a controller from a user creating or updating some association
+      # when they do not have permissions to view the associated items.  An example would be adding random repository ids to
+      # content view.
+      # To support this, within the controller define a method such as:
+      #     def filtered_associations
+      #       {
+      #         :component_ids => Katello::ContentViewVersion,
+      #         :repository_ids => Katello::Repository
+      #       }
+      #     end
+      # This assumes that the parameters are 'wrapped'.  So the above in the content_views_controller, actually looks at
+      #  a subhash of 'content_view'
+
+      included do
+        before_action :check_association_ids, :only => [:create, :update]
+      end
+
+      def check_association_ids
+        if filtered_associations
+          wrapped_params = params[self._wrapper_options.name]
+          find_param_arrays(wrapped_params).each do |key_path|
+            if (model_class = filtered_associations.with_indifferent_access.dig(*key_path))
+              param_ids = wrapped_params.dig(*key_path)
+              filtered_ids = model_class.readable.where(:id => param_ids).pluck(:id)
+              if (unfound_ids = param_ids_missing(param_ids, filtered_ids)).any?
+                fail HttpErrors::NotFound, _("One or more ids (%{ids}) were not found for %{assoc}.  You may not have permissions to see them.") %
+                    {ids: unfound_ids, assoc: key_path.last}
+              end
+            else
+              fail _("Unfiltered params array: %s.") % key_path
+            end
+          end
+        else
+          Rails.logger.warn("#{self.class.name} may has unprotected associations, see associations_permission_check.rb for details.") if ENV['RAILS_ENV'] == 'development'
+        end
+      end
+
+      def filtered_associations
+        #should return {} when supported by all controllers
+        nil
+      end
+
+      def param_ids_missing(param_ids, filtered_ids)
+        param_ids.map(&:to_i).uniq - filtered_ids.map(&:to_i).uniq
+      end
+
+      #returns an array of list of keys pointing to an array in a params hash i.e.:
+      # {"a"=> {"b" => [3]}}  =>  [["a", "b"]]
+      def find_param_arrays(hash = params)
+        list_of_paths = []
+        hash.each do |key, value|
+          if value.is_a?(ActionController::Parameters) || value.is_a?(Hash)
+            list_of_paths += find_param_arrays(value).compact.map { |inner_keys| [key] + inner_keys }
+          elsif value.is_a?(Array)
+            list_of_paths << [key]
+          end
+        end
+        list_of_paths.compact
+      end
+    end
+  end
+end

@@ -0,0 +1,78 @@
+# encoding: utf-8
+
+require "katello_test_helper"
+
+module Katello
+  class TestAssociationIdController
+    def initialize(params, filtered_associations)
+      @params = params
+      @filtered_associations = filtered_associations
+    end
+
+    def self.before_action(*_args)
+    end
+
+    include Concerns::Api::V2::AssociationsPermissionCheck
+
+    attr_accessor :filtered_associations
+    attr_reader :params
+
+    def _wrapper_options
+      OpenStruct.new(:name => :content_view)
+    end
+  end
+
+  class Api::V2::AssociationsPermissionCheckTest < ActiveSupport::TestCase
+    def setup
+      @cv = katello_content_views(:acme_default)
+      @repo = katello_repositories(:fedora_17_x86_64)
+
+      @params = {
+        content_view: {
+          foo: [@cv.id],
+          foo2: 3,
+          foo3: {
+            baz: [@repo.id],
+            baz2: 9
+          }
+        }
+      }
+
+      @filtered_associations = {
+        foo: ::Katello::ContentView,
+        foo3: {
+          baz: ::Katello::Repository
+        }
+      }
+    end
+
+    def test_find_param_arrays
+      controller = TestAssociationIdController.new(@params, @filtered_associations)
+      assert_equal [[:content_view, :foo], [:content_view, :foo3, :baz]].sort, controller.find_param_arrays.sort
+    end
+
+    def test_check_association_ids_positive
+      controller = TestAssociationIdController.new(@params, @filtered_associations)
+
+      controller.check_association_ids
+    end
+
+    def test_check_association_ids_not_found_id
+      @params[:content_view][:foo] << -1
+      controller = TestAssociationIdController.new(@params, @filtered_associations)
+
+      assert_raises(Katello::HttpErrors::NotFound) do
+        controller.check_association_ids
+      end
+    end
+
+    def test_check_association_ids_not_defined
+      @params[:content_view][:not_defined] = [1]
+      controller = TestAssociationIdController.new(@params, @filtered_associations)
+
+      assert_raises(StandardError) do
+        controller.check_association_ids
+      end
+    end
+  end
+end

@@ -125,6 +125,30 @@ def test_create_protected
       end
     end
 
+    def test_create_protected_without_repo_read
+      user = User.unscoped.find(users(:restricted).id)
+      denied_perms = [@create_permission, @view_permission, @update_permission, :destroy_content_views]
+      setup_user_with_permissions(denied_perms, user)
+      repository = katello_repositories(:fedora_17_unpublished)
+      login_user(user)
+
+      post :create, params: {:organization_id => @organization.id, content_view: { :name => "Test", :repository_ids => [repository.id] } }
+
+      assert_response 404
+    end
+
+    def test_create_protected_without_repo_read_non_wrapped
+      user = User.unscoped.find(users(:restricted).id)
+      denied_perms = [@create_permission, @view_permission, @update_permission, :destroy_content_views]
+      setup_user_with_permissions(denied_perms, user)
+      repository = katello_repositories(:fedora_17_unpublished)
+
+      login_user(user)
+      post :create, params: { :name => "Test", :organization_id => @organization.id, :repository_ids => [repository.id] }
+
+      assert_response 404
+    end
+
     def test_create_with_non_json_request
       @request.env['CONTENT_TYPE'] = 'application/x-www-form-urlencoded'
       post :create, params: { :name => "My View", :description => "Cool", :organization_id => @organization.id }
@@ -175,6 +199,21 @@ def test_update
     def test_update_repositories
       repository = katello_repositories(:fedora_17_unpublished)
 
+      params = { :repository_ids => [repository.id] }
+      assert_sync_task(::Actions::Katello::ContentView::Update) do |_content_view, content_view_params|
+        content_view_params.key?(:repository_ids).must_equal true
+        content_view_params[:repository_ids].must_equal params[:repository_ids]
+      end
+      put :update, params: { :id => @library_dev_staging_view.id, :content_view => params }
+
+      assert_response :success
+      assert_template layout: 'katello/api/v2/layouts/resource'
+      assert_template 'katello/api/v2/common/update'
+    end
+
+    def test_update_repositories_strings
+      repository = katello_repositories(:fedora_17_unpublished)
+
       params = { :repository_ids => [repository.id.to_s] }
       assert_sync_task(::Actions::Katello::ContentView::Update) do |_content_view, content_view_params|
         content_view_params.key?(:repository_ids).must_equal true
@@ -191,7 +230,7 @@ def test_update_components
       version = @library_dev_staging_view.versions.first
       composite = ContentView.find(katello_content_views(:composite_view).id)
 
-      params = { :component_ids => [version.id.to_s] }
+      params = { :component_ids => [version.id] }
       assert_sync_task(::Actions::Katello::ContentView::Update) do |_content_view, content_view_params|
         content_view_params.key?(:component_ids).must_equal true
         content_view_params[:component_ids].must_equal params[:component_ids]