From b8c02f14c5b0386a87adfd1ccb815472442ce037 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Gaya?= <sebastien.gaya@gmail.com>
Date: Thu, 7 Dec 2023 17:41:29 +0100
Subject: [PATCH] whitelist where we use constantize on user input
---
app/assets/javascripts/admin/commons/association.js | 4 ++--
app/controllers/admin/application_controller.rb | 4 ++++
.../admin/communication/blocks/headings_controller.rb | 2 +-
.../admin/communication/blocks_controller.rb | 6 +++---
.../admin/communication/contents_controller.rb | 2 +-
.../communication/extranets/contacts_controller.rb | 4 +---
.../admin/communication/websites/pages_controller.rb | 4 +---
.../communication/websites/permalinks_controller.rb | 3 ++-
app/controllers/admin/users_controller.rb | 5 ++---
app/models/communication/extranet/connection.rb | 4 ++++
app/models/communication/website/page.rb | 4 ++++
app/models/communication/website/page/organization.rb | 4 ++++
app/models/communication/website/page/person.rb | 4 ++++
app/services/polymorphic_object_finder.rb | 10 ++++++----
.../communication/extranets/contacts/_toggle.html.erb | 4 ++--
.../pages/show/special_pages/_organization.html.erb | 2 +-
.../websites/pages/show/special_pages/_person.html.erb | 2 +-
17 files changed, 43 insertions(+), 25 deletions(-)
diff --git a/app/assets/javascripts/admin/commons/association.js b/app/assets/javascripts/admin/commons/association.js
index b4a0fcd5e..e93455cbb 100644
--- a/app/assets/javascripts/admin/commons/association.js
+++ b/app/assets/javascripts/admin/commons/association.js
@@ -10,8 +10,8 @@ $(function () {
type: 'POST',
url: target,
data: {
- objectId: id,
- objectType: type
+ 'object_id': id,
+ 'object_type': type
}
}).done(function () {
location.reload();
diff --git a/app/controllers/admin/application_controller.rb b/app/controllers/admin/application_controller.rb
index 5a98ff54f..3861751d2 100644
--- a/app/controllers/admin/application_controller.rb
+++ b/app/controllers/admin/application_controller.rb
@@ -38,6 +38,10 @@ class Admin::ApplicationController < ApplicationController
# If the block doesn't exist anymore
end
+ def model_names_with_blocks
+ @model_names_with_blocks ||= ApplicationRecord.descendants.select { |model| model.included_modules.include?(WithBlocks) }.map(&:name)
+ end
+
private
def redirect_if_context_is_not_an_university!
diff --git a/app/controllers/admin/communication/blocks/headings_controller.rb b/app/controllers/admin/communication/blocks/headings_controller.rb
index 53e597801..11114a04c 100644
--- a/app/controllers/admin/communication/blocks/headings_controller.rb
+++ b/app/controllers/admin/communication/blocks/headings_controller.rb
@@ -20,7 +20,7 @@ class Admin::Communication::Blocks::HeadingsController < Admin::Communication::B
end
def new
- @heading.about = PolymorphicObjectFinder.find params, :about
+ @heading.about = PolymorphicObjectFinder.find(params, :about, current_university, whitelist: model_names_with_blocks)
breadcrumb
end
diff --git a/app/controllers/admin/communication/blocks_controller.rb b/app/controllers/admin/communication/blocks_controller.rb
index 4baa5141c..e2b24df08 100644
--- a/app/controllers/admin/communication/blocks_controller.rb
+++ b/app/controllers/admin/communication/blocks_controller.rb
@@ -18,7 +18,7 @@ class Admin::Communication::BlocksController < Admin::Communication::Application
end
def new
- @block.about = PolymorphicObjectFinder.find params, :about
+ @block.about = PolymorphicObjectFinder.find(params, :about, current_university, whitelist: model_names_with_blocks)
breadcrumb
end
@@ -61,12 +61,12 @@ class Admin::Communication::BlocksController < Admin::Communication::Application
return unless request.xhr?
cookies.signed[Communication::Block::BLOCK_COPY_COOKIE] = {
value: params[:id],
- path: '/admin'
+ path: '/admin'
}
end
def paste
- about = PolymorphicObjectFinder.find(params, :about)
+ about = PolymorphicObjectFinder.find(params, :about, current_university, whitelist: model_names_with_blocks)
# On réattribue à @block pour bénéficier du calcul dans about_path
@block = @block.paste(about)
cookies.delete(Communication::Block::BLOCK_COPY_COOKIE, path: '/admin')
diff --git a/app/controllers/admin/communication/contents_controller.rb b/app/controllers/admin/communication/contents_controller.rb
index ff344cda5..bdd741ef2 100644
--- a/app/controllers/admin/communication/contents_controller.rb
+++ b/app/controllers/admin/communication/contents_controller.rb
@@ -13,7 +13,7 @@ class Admin::Communication::ContentsController < Admin::Communication::Applicati
protected
def load_about
- @about = PolymorphicObjectFinder.find(params, :about)
+ @about = PolymorphicObjectFinder.find(params, :about, current_university, whitelist: model_names_with_blocks)
raise_403_unless @about.university == current_university
raise_403_unless can?(:edit, @about)
end
diff --git a/app/controllers/admin/communication/extranets/contacts_controller.rb b/app/controllers/admin/communication/extranets/contacts_controller.rb
index 6283e05b3..417447132 100644
--- a/app/controllers/admin/communication/extranets/contacts_controller.rb
+++ b/app/controllers/admin/communication/extranets/contacts_controller.rb
@@ -53,8 +53,6 @@ class Admin::Communication::Extranets::ContactsController < Admin::Communication
protected
def load_object
- object_type = params[:objectType]
- object_id = params[:objectId]
- @object = object_type.constantize.find object_id
+ @object = PolymorphicObjectFinder.find(params, :object, current_university, whitelist: Communication::Extranet::Connection.connectable_model_names)
end
end
diff --git a/app/controllers/admin/communication/websites/pages_controller.rb b/app/controllers/admin/communication/websites/pages_controller.rb
index f788465d0..aae55ee3a 100644
--- a/app/controllers/admin/communication/websites/pages_controller.rb
+++ b/app/controllers/admin/communication/websites/pages_controller.rb
@@ -134,9 +134,7 @@ class Admin::Communication::Websites::PagesController < Admin::Communication::We
protected
def load_object
- object_type = params[:objectType]
- object_id = params[:objectId]
- @object = object_type.constantize.find object_id
+ @object = PolymorphicObjectFinder.find(params, :object, current_university, whitelist: @page.connectable_model_names)
end
def breadcrumb
diff --git a/app/controllers/admin/communication/websites/permalinks_controller.rb b/app/controllers/admin/communication/websites/permalinks_controller.rb
index 49a39f0c4..28aec9d20 100644
--- a/app/controllers/admin/communication/websites/permalinks_controller.rb
+++ b/app/controllers/admin/communication/websites/permalinks_controller.rb
@@ -2,7 +2,8 @@ class Admin::Communication::Websites::PermalinksController < Admin::Communicatio
def create
@path = params['communication_website_permalink']['path']
- @about = PolymorphicObjectFinder.find(params, :about)
+ model_names_with_permalinks = ApplicationRecord.descendants.select { |model| model.included_modules.include?(WithPermalink) }.map(&:name)
+ @about = PolymorphicObjectFinder.find(params, :about, current_university, whitelist: model_names_with_permalinks)
@permalink = @about.add_redirection(@path)
end
end
\ No newline at end of file
diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb
index 122be9729..3968a1e2a 100644
--- a/app/controllers/admin/users_controller.rb
+++ b/app/controllers/admin/users_controller.rb
@@ -21,9 +21,8 @@ class Admin::UsersController < Admin::ApplicationController
def favorite
operation = params[:operation]
- id = params[:about_id]
- type = params[:about_type]
- about = type.constantize.find id
+ favoritable_model_names = ApplicationRecord.descendants.select { |model| model.included_modules.include?(Favoritable) }.map(&:name)
+ about = PolymorphicObjectFinder.find(params, :about, current_university, whitelist: favoritable_model_names)
if operation == 'add'
current_user.add_favorite(about)
else
diff --git a/app/models/communication/extranet/connection.rb b/app/models/communication/extranet/connection.rb
index 30cc10e73..9d06bb3c9 100644
--- a/app/models/communication/extranet/connection.rb
+++ b/app/models/communication/extranet/connection.rb
@@ -25,4 +25,8 @@ class Communication::Extranet::Connection < ApplicationRecord
belongs_to :university
belongs_to :extranet, class_name: 'Communication::Extranet'
belongs_to :object, polymorphic: true
+
+ def self.connectable_model_names
+ ["University::Organization", "University::Person"]
+ end
end
diff --git a/app/models/communication/website/page.rb b/app/models/communication/website/page.rb
index 6ddca2a2d..82c736ab2 100644
--- a/app/models/communication/website/page.rb
+++ b/app/models/communication/website/page.rb
@@ -142,6 +142,10 @@ class Communication::Website::Page < ApplicationRecord
.where.not(id: id)
end
+ def connectable_model_names
+ []
+ end
+
protected
def check_accessibility
diff --git a/app/models/communication/website/page/organization.rb b/app/models/communication/website/page/organization.rb
index b17a2c96d..3b9249339 100644
--- a/app/models/communication/website/page/organization.rb
+++ b/app/models/communication/website/page/organization.rb
@@ -57,6 +57,10 @@ class Communication::Website::Page::Organization < Communication::Website::Page
University::Organization.where(id: ids)
end
+ def connectable_model_names
+ ["University::Organization"]
+ end
+
protected
def current_git_path
diff --git a/app/models/communication/website/page/person.rb b/app/models/communication/website/page/person.rb
index 4c11cd187..431a660a3 100644
--- a/app/models/communication/website/page/person.rb
+++ b/app/models/communication/website/page/person.rb
@@ -56,6 +56,10 @@ class Communication::Website::Page::Person < Communication::Website::Page
University::Person.where(id: ids)
end
+ def connectable_model_names
+ ["University::Person"]
+ end
+
protected
def current_git_path
diff --git a/app/services/polymorphic_object_finder.rb b/app/services/polymorphic_object_finder.rb
index 47bf78dcc..763cba5a1 100644
--- a/app/services/polymorphic_object_finder.rb
+++ b/app/services/polymorphic_object_finder.rb
@@ -1,12 +1,14 @@
class PolymorphicObjectFinder
- # @block.about = Polymorphic.find params, :about
+ # @block.about = Polymorphic.find params, :about, current_university, whitelist: ["Communication::Website::Page"]
# Rails uses ActiveRecord::Inheritance#polymorphic_name to hydrate the about_type.
# Example: A Block for a Communication::Website::Page::Home will have about_type = "Communication::Website::Page"
- def self.find(params, key)
+ def self.find(params, key, university, whitelist: [])
key_id = "#{key}_id".to_sym
key_type = "#{key}_type".to_sym
- klass = params[key_type].constantize
+ model_name = whitelist.detect { |item| item == params[key_type] }
+ return unless model_name.nil?
+ model = model.constantize
id = params[key_id]
- klass.find id
+ model.where(university: university).find(id)
end
end
\ No newline at end of file
diff --git a/app/views/admin/communication/extranets/contacts/_toggle.html.erb b/app/views/admin/communication/extranets/contacts/_toggle.html.erb
index 0d8b6960e..23b704726 100644
--- a/app/views/admin/communication/extranets/contacts/_toggle.html.erb
+++ b/app/views/admin/communication/extranets/contacts/_toggle.html.erb
@@ -2,8 +2,8 @@
connected = @extranet.connected?(about)
path = toggle_admin_communication_extranet_contacts_path(
extranet_id: @extranet.id,
- objectId: about.id,
- objectType: about.class,
+ object_id: about.id,
+ object_type: about.class,
)
%>
<input class="form-check-input"
diff --git a/app/views/admin/communication/websites/pages/show/special_pages/_organization.html.erb b/app/views/admin/communication/websites/pages/show/special_pages/_organization.html.erb
index 9292423bb..232e8ca99 100644
--- a/app/views/admin/communication/websites/pages/show/special_pages/_organization.html.erb
+++ b/app/views/admin/communication/websites/pages/show/special_pages/_organization.html.erb
@@ -22,7 +22,7 @@
<tr>
<td><%= link_to organization, [:admin, organization] %></td>
<td><%= link_to 'Déconnecter',
- disconnect_admin_communication_website_page_path(@page, objectId: organization.id, objectType: organization.class),
+ disconnect_admin_communication_website_page_path(@page, object_id: organization.id, object_type: organization.class),
class: button_classes_danger,
method: :post %></td>
</tr>
diff --git a/app/views/admin/communication/websites/pages/show/special_pages/_person.html.erb b/app/views/admin/communication/websites/pages/show/special_pages/_person.html.erb
index 0c263addf..babffb39e 100644
--- a/app/views/admin/communication/websites/pages/show/special_pages/_person.html.erb
+++ b/app/views/admin/communication/websites/pages/show/special_pages/_person.html.erb
@@ -22,7 +22,7 @@
<tr>
<td><%= link_to person, [:admin, person] %></td>
<td><%= link_to 'Déconnecter',
- disconnect_admin_communication_website_page_path(@page, objectId: person.id, objectType: person.class),
+ disconnect_admin_communication_website_page_path(@page, object_id: person.id, object_type: person.class),
class: button_classes_danger,
method: :post %></td>
</tr>
--
GitLab