Skip to content

Commit

Permalink
security: prevent SQL injection in sorting param (#2756)
Browse files Browse the repository at this point in the history
* security: sorting

* extract sorting to own method

* get_field

* detail

* fix

* fix

* refactor complexity

* lint

* fix

* fix Model to ActiveRecord::Relation
  • Loading branch information
Paul-Bob authored May 9, 2024
1 parent 03aa576 commit 34e69b7
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 20 deletions.
2 changes: 1 addition & 1 deletion app/components/avo/base_component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ def has_with_trial(ability)
# Use the @parent_resource to fetch the field using the @reflection name.
def field
reflection_name = params[:related_name]&.to_sym || @reflection.name
@parent_resource.get_field_definitions.find { |f| f.id == reflection_name }
@parent_resource.get_field(reflection_name)
rescue
nil
end
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/avo/associations_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ def set_attachment_record
end

def set_reflection_field
@field = @resource.get_field_definitions.find { |f| f.id == @related_resource_name.to_sym }
@field = @resource.get_field(@related_resource_name.to_sym)
@field.hydrate(resource: @resource, record: @record, view: :new)
rescue
end
Expand Down
44 changes: 29 additions & 15 deletions app/controllers/avo/base_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,21 +34,7 @@ def index
@query = @query.includes(*@resource.includes)
end

# Sort the items
if @index_params[:sort_by].present?
unless @index_params[:sort_by].eql? :created_at
@query = @query.unscope(:order)
end

# Check if the sortable field option is actually a proc and we need to do a custom sort
field_id = @index_params[:sort_by].to_sym
field = @resource.get_field_definitions.find { |field| field.id == field_id }
@query = if field&.sortable.is_a?(Proc)
Avo::ExecutionContext.new(target: field.sortable, query: @query, direction: @index_params[:sort_direction]).handle
else
@query.order("#{@resource.model_class.table_name}.#{@index_params[:sort_by]} #{@index_params[:sort_direction]}")
end
end
apply_sorting

# Apply filters to the current query
filters_to_be_applied.each do |filter_class, filter_value|
Expand Down Expand Up @@ -600,5 +586,33 @@ def set_component_for(view, fallback_view: nil)
def apply_pagination
@pagy, @records = @resource.apply_pagination(index_params: @index_params, query: pagy_query)
end

def apply_sorting
return if @index_params[:sort_by].nil?

sort_by = @index_params[:sort_by].to_sym
if sort_by != :created_at
@query = @query.unscope(:order)
end

# Verify that sort_by param actually is bonded to a field.
field = @resource.get_field(sort_by)

# Check if the sortable field option is a proc and if there is a need to do a custom sort
@query = if field.present? && field.sortable.is_a?(Proc)
Avo::ExecutionContext.new(target: field.sortable, query: @query, direction: sanitized_sort_direction).handle
# Sanitize sort_by param by checking if have bounded field.
elsif field.present? && sanitized_sort_direction
@query.order("#{@resource.model_class.table_name}.#{sort_by} #{sanitized_sort_direction}")
# Transform Model to ActiveRecord::Relation because Avo expects one.
else
@query.where("1=1")
end
end

# Sanitize sort_direction param
def sanitized_sort_direction
@sanitized_sort_direction ||= @index_params[:sort_direction].presence_in(["asc", "desc"])
end
end
end
4 changes: 1 addition & 3 deletions app/controllers/avo/search_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -173,9 +173,7 @@ def fetch_field
user: _current_user
)

reflection_resource.detect_fields.get_field_definitions.find do |field|
field.id.to_s == params[:via_association_id]
end
reflection_resource.detect_fields.get_field(params[:via_association_id])
end

def fetch_parent
Expand Down

0 comments on commit 34e69b7

Please sign in to comment.