From caffb7e8a60004c9c536d594ff767be33046f372 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Hul=C3=A1n?= Date: Wed, 9 Nov 2016 11:30:39 +0100 Subject: [PATCH] Fixes #16971 - CVE-2016-7077 remove unauthorized checkboxes --- app/helpers/application_helper.rb | 12 ------------ app/helpers/form_helper.rb | 14 ++++---------- app/views/common/_edit_habtm.html.erb | 21 --------------------- test/integration/operatingsystem_test.rb | 2 +- test/integration/puppetclass_test.rb | 6 +++--- 5 files changed, 8 insertions(+), 47 deletions(-) delete mode 100644 app/views/common/_edit_habtm.html.erb diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index e9f10538d3b..6c9cd8160ee 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -50,11 +50,6 @@ def show_habtm(associations) render :partial => 'common/show_habtm', :collection => associations, :as => :association end - def edit_habtm(klass, association, prefix = nil, options = {}) - render :partial => 'common/edit_habtm', :locals =>{:prefix => prefix, :klass => klass, :options => options, - :associations => association.all.sort.delete_if{|e| e == klass}} - end - def link_to_remove_puppetclass(klass, type) options = options_for_puppetclass_selection(klass, type) text = remove_link_to_function(truncate(klass.name, :length => 28), options) @@ -156,13 +151,6 @@ def new_link(name, options = {}, html_options = {}) display_link_if_authorized(name, options, html_options) end - def authorized_edit_habtm(klass, association, prefix = nil, options = {}) - if authorized_for :controller => params[:controller], :action => params[:action] - return edit_habtm(klass, association, prefix, options) - end - show_habtm klass.send(association.name.pluralize.downcase) - end - # renders a style=display based on an attribute properties def display?(attribute = true) "style=#{display(attribute)}" diff --git a/app/helpers/form_helper.rb b/app/helpers/form_helper.rb index 1cf9acc9b54..1c817f451de 100644 --- a/app/helpers/form_helper.rb +++ b/app/helpers/form_helper.rb @@ -46,15 +46,9 @@ def checkbox_f(f, attr, options = {}, checked_value = "1", unchecked_value = "0" end def multiple_checkboxes(f, attr, klass, associations, options = {}, html_options = {}) - if associations.count > 5 - associated_obj = klass.send(ActiveModel::Naming.plural(associations.first)) - selected_ids = associated_obj.select("#{associations.first.class.table_name}.id").map(&:id) - multiple_selects(f, attr, associations, selected_ids, options, html_options) - else - field(f, attr, options) do - authorized_edit_habtm klass, associations, options[:prefix], html_options - end - end + associated_obj = klass.send(ActiveModel::Naming.plural(associations.new)) + selected_ids = associated_obj.select("#{associations.new.class.table_name}.id").map(&:id) + multiple_selects(f, attr, associations, selected_ids, options, html_options) end # add hidden field for options[:disabled] @@ -68,7 +62,7 @@ def multiple_selects(f, attr, associations, selected_ids, options = {}, html_opt else klass = nil end - authorized = AssociationAuthorizer.authorized_associations(associations, klass).all + authorized = AssociationAuthorizer.authorized_associations(associations.reorder(nil), klass).all # select2.js breaks the multiselects disabled items location # http://projects.theforeman.org/issues/12028 diff --git a/app/views/common/_edit_habtm.html.erb b/app/views/common/_edit_habtm.html.erb deleted file mode 100644 index 17d574e7649..00000000000 --- a/app/views/common/_edit_habtm.html.erb +++ /dev/null @@ -1,21 +0,0 @@ -<% if associations.empty? %> - <%= _("None Found") %> -<% else %> - <% association = associations.first %> - <%= link_to_function(icon_text("check", _("Select All")), - "toggleCheckboxesBySelector(\"[id$='#{ActiveModel::Naming.singular(association)}_ids_']\")" ) %> - -<% end %> diff --git a/test/integration/operatingsystem_test.rb b/test/integration/operatingsystem_test.rb index 1905f48468c..7a2d91cc549 100644 --- a/test/integration/operatingsystem_test.rb +++ b/test/integration/operatingsystem_test.rb @@ -11,7 +11,7 @@ class OperatingsystemIntegrationTest < ActionDispatch::IntegrationTest fill_in "operatingsystem_major", :with => "9" fill_in "operatingsystem_minor", :with => "2" select "Arch Linux", :from => "operatingsystem_family" - check "x86_64" + select "x86_64", :from => "operatingsystem_architecture_ids" assert_submit_button(operatingsystems_path) assert page.has_link? "Archy 9.2" end diff --git a/test/integration/puppetclass_test.rb b/test/integration/puppetclass_test.rb index 75e95b00136..268f490d64f 100644 --- a/test/integration/puppetclass_test.rb +++ b/test/integration/puppetclass_test.rb @@ -1,11 +1,11 @@ require 'integration_test_helper' -class PuppetclassIntegrationTest < ActionDispatch::IntegrationTest +class PuppetclassIntegrationTest < IntegrationTestWithJavascript test "edit page" do visit puppetclasses_path click_link "vim" - refute page.has_link? 'Common' - click_link "Select All" + assert page.has_no_link? 'Common' + find(:xpath, "//a[@data-original-title='Select All']").click assert_submit_button(puppetclasses_path) assert page.has_link? 'vim' assert page.has_link? 'Common'