CVE-2020-36190: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting') (CWE-79)
value: RailsAdmin (aka rails_admin) before 1.4.3 and 2.x before 2.0.2 allows XSS via nested forms.
CWE description: The product does not neutralize or incorrectly neutralizes user-controllable input before it is placed in output that is used as a web page that is served to other users.
CVSS v3 base score: 6.1
Published: 2021-01-12T20:15Z
Repository: https://github.com/sferik/rails_admin
Language: Ruby
Files changed: app/assets/javascripts/rails_admin/ra.nested-form-hooks.js, app/assets/javascripts/rails_admin/ra.widgets.js, spec/integration/fields/has_many_association_spec.rb, spec/integration/fields/has_one_association_spec.rb
Commit date: 2020-03-14 18:14:25 +0900
Commit message: Fix XSS vulnerability in nested forms

Diff: 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 a7509a891..8175335d4 100644
--- a/app/assets/javascripts/rails_admin/ra.nested-form-hooks.js
+++ b/app/assets/javascripts/rails_admin/ra.nested-form-hooks.js
@@ -9,11 +9,13 @@
   });
 
   $(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 = $('<li><a data-toggle="tab" href="#' + field.attr('id') + '">' + field.children('.object-infos').data('object-label') + '</a></li>');
+    new_tab = $('<li></li>').append(
+        $('<a></a>').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;
     nav = controls.children('.nav');
     content = parent_group.children('.tab-content');
@@ -25,11 +27,11 @@
       nav.select(':hidden').show('slow');
     }
     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'));
     }
   });
 
   $(document).on('nested:fieldRemoved', 'form', function(content) {
     var add_button, controls, current_li, field, nav, one_to_one, parent_group, toggler;
diff --git a/app/assets/javascripts/rails_admin/ra.widgets.js b/app/assets/javascripts/rails_admin/ra.widgets.js
index b3e6e9aa4..4662e2bca 100644
--- a/app/assets/javascripts/rails_admin/ra.widgets.js
+++ b/app/assets/javascripts/rails_admin/ra.widgets.js
@@ -135,11 +135,15 @@
         nav = field.find('> .controls > .nav');
         tab_content = field.find('> .tab-content');
         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('<li><a data-toggle="tab" href="#' + this.id + '">' + $(this).children('.object-infos').data('object-label') + '</a></li>');
+          nav.append(
+              $('<li></li>').append(
+                  $('<a></a>').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');
         }
         if (nav.children().length === 0) {
@@ -163,12 +167,16 @@
         field = $(this).parents('.control-group').first();
         nav = field.find("> .controls > .nav");
         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('<li><a data-toggle="tab" href="#' + this.id + '">' + $(this).children('.object-infos').data('object-label') + '</a></li>');
+          field.find('> .controls .add_nested_fields').removeClass('add_nested_fields').text($(this).children('.object-infos').data('object-label'));
+          nav.append(
+              $('<li></li>').append(
+                  $('<a></a>').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');
         field.find("> .controls > [data-target]:first").html('<i class="icon-white"></i> ' + first_tab.html());
         nav.hide();
diff --git a/spec/integration/fields/has_many_association_spec.rb b/spec/integration/fields/has_many_association_spec.rb
index 432790575..525ee385a 100644
--- a/spec/integration/fields/has_many_association_spec.rb
+++ b/spec/integration/fields/has_many_association_spec.rb
@@ -209,10 +209,26 @@ RSpec.describe 'HasManyAssociation field', type: :request do
         visit new_path(model_name: 'field_test')
         expect(page.body).to_not include('field_test_nested_field_tests_attributes_new_nested_field_tests_deeply_nested_field_tests_attributes_new_deeply_nested_field_tests_nested_field_test_id_field')
         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('<script>throw "XSS";</script>')
+        @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: '<script>throw "XSS";</script>', 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
     before do
       RailsAdmin.config FieldTest 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 ef681f9e9..b2ee3f499 100644
--- a/spec/integration/fields/has_one_assosiation_spec.rb
+++ b/spec/integration/fields/has_one_association_spec.rb
@@ -89,10 +89,29 @@ RSpec.describe 'HasOneAssociation field', type: :request do
       visit edit_path(model_name: 'field_test', id: @record.id)
       click_button 'Save'
       @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('<script>throw "XSS";</script>')
+        @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: '<script>throw "XSS";</script>', commentable: @record
+        visit edit_path(model_name: 'field_test', id: @record.id)
+      end
+    end
   end
 
   context 'with custom primary_key option' do
     let(:user) { FactoryBot.create :managing_user }
     let!(:team) { FactoryBot.create(:managed_team) }

Vulnerable code:     new_tab = $('<li><a data-toggle="tab" href="#' + field.attr('id') + '">' + field.children('.object-infos').data('object-label') + '</a></li>');
      controls.find('.add_nested_fields').removeClass('add_nested_fields').html(field.children('.object-infos').data('object-label'));
          nav.append('<li><a data-toggle="tab" href="#' + this.id + '">' + $(this).children('.object-infos').data('object-label') + '</a></li>');
          field.find('> .controls .add_nested_fields').removeClass('add_nested_fields').html($(this).children('.object-infos').data('object-label'));
          nav.append('<li><a data-toggle="tab" href="#' + this.id + '">' + $(this).children('.object-infos').data('object-label') + '</a></li>');
Fixed code:     new_tab = $('<li></li>').append(
        $('<a></a>').attr('data-toggle', 'tab').attr('href', '#' + field.attr('id')).text(field.children('.object-infos').data('object-label'))
    )
      controls.find('.add_nested_fields').removeClass('add_nested_fields').text(field.children('.object-infos').data('object-label'));
          nav.append(
              $('<li></li>').append(
                  $('<a></a>').attr('data-toggle', 'tab').attr('href', '#' + this.id).text($(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(
              $('<li></li>').append(
                  $('<a></a>').attr('data-toggle', 'tab').attr('href', '#' + this.id).text($(this).children('.object-infos').data('object-label'))
              )
          );

    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('<script>throw "XSS";</script>')
        @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: '<script>throw "XSS";</script>', field_test: @record
        visit edit_path(model_name: 'field_test', id: @record.id)
      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: 'Comment').and_return('<script>throw "XSS";</script>')
        @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: '<script>throw "XSS";</script>', commentable: @record
        visit edit_path(model_name: 'field_test', id: @record.id)
      end
    end