Skip to content

Commit

Permalink
explicit avoid code injection via media_route params (#3966)
Browse files Browse the repository at this point in the history
* explicit avoid code injection via media_route params

media routes declare allowed polymorphic resources for use in routes and these are constrained to certain resource names, https://github.com/zooniverse/panoptes/blob/5b0cbf938e1e33830ec4204778aa0c6c9f5030bc/lib/routes/json_api_routes.rb#L34

however to avoid any possible code injection we avoid using those params and instead rely on an allow list

* add profile_header media resource
  • Loading branch information
camallen authored Oct 13, 2022
1 parent 5b0cbf9 commit d4915cb
Showing 1 changed file with 25 additions and 6 deletions.
31 changes: 25 additions & 6 deletions app/controllers/api/v1/media_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,17 @@ class Api::V1::MediaController < Api::ApiController

schema_type :json_schema

ALLOW_LIST_MEDIA_RESOURCES = {
avatar: :avatar,
background: :background,
attached_images: :attached_images,
classifications_export: :classifications_export,
subjects_export: :subjects_export,
workflows_export: :workflows_export,
workflow_contents_export: :workflow_contents_export,
profile_header: :profile_header
}.freeze

def schema_class(action)
"medium_#{ action }_schema".camelize.constantize
end
Expand All @@ -29,12 +40,12 @@ def create

created_media_resource = Medium.transaction(requires_new: true) do
if many_association?
polymorphic_controlled_resourse.send(media_name).create!(create_params)
polymorphic_controlled_resourse.send(media_name_method).create!(create_params)
else
if (old_resource = polymorphic_controlled_resourse.send(media_name))
if (old_resource = polymorphic_controlled_resourse.send(media_name_method))
old_resource.destroy
end
polymorphic_controlled_resourse.send("create_#{media_name}!", create_params)
polymorphic_controlled_resourse.send("create_#{media_name_method}!", create_params)
end
end

Expand Down Expand Up @@ -79,15 +90,15 @@ def controlled_scope

def raise_no_resources_error
raise Api::NoMediaError.new(
media_name,
media_name_method,
polymorphic_klass_name,
polymorphic_ids,
params[:id]
)
end

def association_reflection
@association_reflection ||= polymorphic_klass.reflect_on_association(media_name)
@association_reflection ||= polymorphic_klass.reflect_on_association(media_name_method)
end

def one_association?
Expand All @@ -110,6 +121,14 @@ def controlled_resources
# attached images have where(type: "tutorial_attached_image") } filters
# so these has_many relations need to be singular types
def singular_linked_media_type
"#{polymorphic_klass_name}_#{params[:media_name]}".singularize
"#{polymorphic_klass_name}_#{media_name_method}".singularize
end

# avoid possible code injection from the media_controller routes
# that set the media_name param (e.g. /api/organizations/:id/attached_images)
# in theory this is controlled through our route constraints
# but it's best to avoid relying on that and instead provide an allow list here
def media_name_method
ALLOW_LIST_MEDIA_RESOURCES[media_name.to_sym]
end
end

0 comments on commit d4915cb

Please sign in to comment.