From d72090ec6a07c3b9b7b48ab50f3d405f91ff4375 Mon Sep 17 00:00:00 2001 From: "M.Shibuya" Date: Sat, 14 Mar 2020 18:14:25 +0900 Subject: [PATCH] Fix XSS vulnerability in nested forms --- .../rails_admin/ra.nested-form-hooks.js | 6 ++++-- .../javascripts/rails_admin/ra.widgets.js | 14 +++++++++++--- .../fields/has_many_association_spec.rb | 16 ++++++++++++++++ ...on_spec.rb => has_one_association_spec.rb} | 19 +++++++++++++++++++ 4 files changed, 50 insertions(+), 5 deletions(-) rename spec/integration/fields/{has_one_assosiation_spec.rb => has_one_association_spec.rb} (84%) diff --git a/app/assets/javascripts/rails_admin/ra.nested-form-hooks.js b/app/assets/javascripts/rails_admin/ra.nested-form-hooks.js index a7509a891e..8175335d45 100644 --- a/app/assets/javascripts/rails_admin/ra.nested-form-hooks.js +++ b/app/assets/javascripts/rails_admin/ra.nested-form-hooks.js @@ -11,7 +11,9 @@ $(document).on('nested:fieldAdded', 'form', function(content) { var controls, field, nav, new_tab, one_to_one, parent_group, toggler; field = content.field.addClass('tab-pane').attr('id', 'unique-id-' + (new Date().getTime())); - new_tab = $('
  • ' + field.children('.object-infos').data('object-label') + '
  • '); + new_tab = $('
  • ').append( + $('').attr('data-toggle', 'tab').attr('href', '#' + field.attr('id')).text(field.children('.object-infos').data('object-label')) + ) parent_group = field.closest('.control-group'); controls = parent_group.children('.controls'); one_to_one = controls.data('nestedone') !== void 0; @@ -27,7 +29,7 @@ content.select(':hidden').show('slow'); toggler.addClass('active').removeClass('disabled').children('i').addClass('icon-chevron-down').removeClass('icon-chevron-right'); if (one_to_one) { - controls.find('.add_nested_fields').removeClass('add_nested_fields').html(field.children('.object-infos').data('object-label')); + controls.find('.add_nested_fields').removeClass('add_nested_fields').text(field.children('.object-infos').data('object-label')); } }); diff --git a/app/assets/javascripts/rails_admin/ra.widgets.js b/app/assets/javascripts/rails_admin/ra.widgets.js index b3e6e9aa48..4662e2bcaa 100644 --- a/app/assets/javascripts/rails_admin/ra.widgets.js +++ b/app/assets/javascripts/rails_admin/ra.widgets.js @@ -137,7 +137,11 @@ toggler = field.find('> .controls > .btn-group > .toggler'); tab_content.children('.fields:not(.tab-pane)').addClass('tab-pane').each(function() { $(this).attr('id', 'unique-id-' + (new Date().getTime()) + Math.floor(Math.random() * 100000)); - nav.append('
  • ' + $(this).children('.object-infos').data('object-label') + '
  • '); + nav.append( + $('
  • ').append( + $('').attr('data-toggle', 'tab').attr('href', '#' + this.id).text($(this).children('.object-infos').data('object-label')) + ) + ); }); if (nav.find("> li.active").length === 0) { nav.find("> li > a[data-toggle='tab']:first").tab('show'); @@ -165,8 +169,12 @@ tab_content = field.find("> .tab-content"); toggler = field.find('> .controls > .btn-group > .toggler'); tab_content.children(".fields:not(.tab-pane)").addClass('tab-pane active').each(function() { - field.find('> .controls .add_nested_fields').removeClass('add_nested_fields').html($(this).children('.object-infos').data('object-label')); - nav.append('
  • ' + $(this).children('.object-infos').data('object-label') + '
  • '); + field.find('> .controls .add_nested_fields').removeClass('add_nested_fields').text($(this).children('.object-infos').data('object-label')); + nav.append( + $('
  • ').append( + $('').attr('data-toggle', 'tab').attr('href', '#' + this.id).text($(this).children('.object-infos').data('object-label')) + ) + ); }); first_tab = nav.find("> li > a[data-toggle='tab']:first"); first_tab.tab('show'); diff --git a/spec/integration/fields/has_many_association_spec.rb b/spec/integration/fields/has_many_association_spec.rb index 432790575b..525ee385a9 100644 --- a/spec/integration/fields/has_many_association_spec.rb +++ b/spec/integration/fields/has_many_association_spec.rb @@ -211,6 +211,22 @@ expect(page.body).to include('field_test_nested_field_tests_attributes_new_nested_field_tests_deeply_nested_field_tests_attributes_new_deeply_nested_field_tests_title') end end + + context 'when XSS attack is attempted', js: true do + it 'does not break on adding a new item' do + allow(I18n).to receive(:t).and_call_original + expect(I18n).to receive(:t).with('admin.form.new_model', name: 'Nested field test').and_return('') + @record = FactoryBot.create :field_test + visit edit_path(model_name: 'field_test', id: @record.id) + find('#field_test_nested_field_tests_attributes_field .add_nested_fields').click + end + + it 'does not break on editing an existing item' do + @record = FactoryBot.create :field_test + NestedFieldTest.create! title: '', field_test: @record + visit edit_path(model_name: 'field_test', id: @record.id) + end + end end context 'with not nullable foreign key', active_record: true do diff --git a/spec/integration/fields/has_one_assosiation_spec.rb b/spec/integration/fields/has_one_association_spec.rb similarity index 84% rename from spec/integration/fields/has_one_assosiation_spec.rb rename to spec/integration/fields/has_one_association_spec.rb index ef681f9e9e..b2ee3f499d 100644 --- a/spec/integration/fields/has_one_assosiation_spec.rb +++ b/spec/integration/fields/has_one_association_spec.rb @@ -91,6 +91,25 @@ @record.reload expect(@record.comment).to be_nil end + + context 'when XSS attack is attempted', js: true do + it 'does not break on adding a new item' do + allow(I18n).to receive(:t).and_call_original + expect(I18n).to receive(:t).with('admin.form.new_model', name: 'Comment').and_return('') + @record = FactoryBot.create :field_test + visit edit_path(model_name: 'field_test', id: @record.id) + find('#field_test_comment_attributes_field .add_nested_fields').click + end + + it 'does not break on adding an existing item' do + RailsAdmin.config Comment do + object_label_method :content + end + @record = FactoryBot.create :field_test + FactoryBot.create :comment, content: '', commentable: @record + visit edit_path(model_name: 'field_test', id: @record.id) + end + end end context 'with custom primary_key option' do