From 70b82010a700c1363c85e82048fd2b4976f19436 Mon Sep 17 00:00:00 2001 From: evazion Date: Tue, 18 Aug 2020 12:49:38 -0500 Subject: [PATCH] search: fix info leak when searching nested associations. Fix an exploit in #4553. It was possible to use nested searches to infer the contents of private forum posts. For example: * https://danbooru.donmai.us/users?search[forum_posts][id]=121683&search[forum_posts][body_matches]=h* * https://danbooru.donmai.us/users?search[forum_posts][id]=121683&search[forum_posts][body_matches]=he* * https://danbooru.donmai.us/users?search[forum_posts][id]=121683&search[forum_posts][body_matches]=hel* * https://danbooru.donmai.us/users?search[forum_posts][id]=121683&search[forum_posts][body_matches]=hell* * https://danbooru.donmai.us/users?search[forum_posts][id]=121683&search[forum_posts][body_matches]=hello* The above searches returned the user 'albert', indicating that the private forum post with id 121683 starts with the word 'hello'. By guessing the id of a private forum post (which can be done by searching for gaps in the id sequence), and by guessing text within the post (which can be done by sequentially guessing characters with wildcard searches), one could eventually infer the full text of a private forum post. The fix is to make nested searches only return records that are visible to the current user. --- app/logical/concerns/searchable.rb | 37 ++++++++++++------------ test/functional/users_controller_test.rb | 13 +++++++++ 2 files changed, 32 insertions(+), 18 deletions(-) diff --git a/app/logical/concerns/searchable.rb b/app/logical/concerns/searchable.rb index 449e675bd..98b02e152 100644 --- a/app/logical/concerns/searchable.rb +++ b/app/logical/concerns/searchable.rb @@ -153,12 +153,13 @@ module Searchable # This allows the hash keys to be either strings or symbols indifferent_params = params.try(:with_indifferent_access) || params.try(:to_unsafe_h) raise ArgumentError, "unable to process params" if indifferent_params.nil? + attributes.reduce(all) do |relation, attribute| - relation.search_attribute(attribute, indifferent_params) + relation.search_attribute(attribute, indifferent_params, CurrentUser.user) end end - def search_attribute(name, params) + def search_attribute(name, params, current_user) column = column_for_attribute(name) type = column.type || reflect_on_association(name)&.class_name @@ -171,11 +172,11 @@ module Searchable case type when "User" - search_user_attribute(name, params) + search_user_attribute(name, params, current_user) when "Post" - search_post_attribute(name, params) + search_post_attribute(name, params, current_user) when "Model" - search_polymorphic_attribute(name, params) + search_polymorphic_attribute(name, params, current_user) when :string, :text search_text_attribute(name, params) when :boolean @@ -190,7 +191,7 @@ module Searchable search_array_attribute(name, subtype, params) else raise NotImplementedError, "unhandled attribute type: #{name}" if type.blank? - search_includes(name, params, type) + search_includes(name, params, type, current_user) end end @@ -230,36 +231,36 @@ module Searchable end end - def search_user_attribute(attr, params) + def search_user_attribute(attr, params, current_user) if params["#{attr}_name"].present? where(attr => User.search(name_matches: params["#{attr}_name"]).reorder(nil)) else - search_includes(attr, params, "User") + search_includes(attr, params, "User", current_user) end end - def search_post_attribute(attr, params) + def search_post_attribute(attr, params, current_user) if params["#{attr}_tags_match"] - where(attr => Post.user_tag_match(params["#{attr}_tags_match"]).reorder(nil)) + where(attr => Post.user_tag_match(params["#{attr}_tags_match"], current_user).reorder(nil)) else - search_includes(attr, params, "Post") + search_includes(attr, params, "Post", current_user) end end - def search_includes(attr, params, type) + def search_includes(attr, params, type, current_user) model = Kernel.const_get(type) if params["#{attr}_id"].present? - search_attribute("#{attr}_id", params) + search_attribute("#{attr}_id", params, current_user) elsif params["has_#{attr}"].to_s.truthy? || params["has_#{attr}"].to_s.falsy? search_has_include(attr, params["has_#{attr}"].to_s.truthy?, model) elsif parameter_hash?(params[attr]) - where(attr => model.search(params[attr]).reorder(nil)) + where(attr => model.visible(current_user).search(params[attr]).reorder(nil)) else all end end - def search_polymorphic_attribute(attr, params) + def search_polymorphic_attribute(attr, params, current_user) model_keys = ((model_types || []) & params.keys) # The user can only logically specify one model at a time, so more than that should return no results return none if model_keys.length > 1 @@ -272,15 +273,15 @@ module Searchable return none if params["#{attr}_type"].present? && params["#{attr}_type"] != model_key model_specified = true model = Kernel.const_get(model_key) - relation = relation.where(attr => model.search(params[model_key])) + relation = relation.where(attr => model.visible(current_user).search(params[model_key])) end if params["#{attr}_id"].present? - relation = relation.search_attribute("#{attr}_id", params) + relation = relation.search_attribute("#{attr}_id", params, current_user) end if params["#{attr}_type"].present? && !model_specified - relation = relation.search_attribute("#{attr}_type", params) + relation = relation.search_attribute("#{attr}_type", params, current_user) end relation diff --git a/test/functional/users_controller_test.rb b/test/functional/users_controller_test.rb index 1dc5e62a5..4e4cd8927 100644 --- a/test/functional/users_controller_test.rb +++ b/test/functional/users_controller_test.rb @@ -82,6 +82,19 @@ class UsersControllerTest < ActionDispatch::IntegrationTest should respond_to_search(posts_tags_match: "touhou").with { @uploader } should respond_to_search(posts: {rating: "e"}).with { @other_user } should respond_to_search(inviter: {name: "yukari"}).with { @other_user } + + context "a user with private forum posts" do + setup do + as(@user) do + @private_post = create(:forum_post, body: "private", creator: @user, topic: create(:mod_up_forum_topic)) + @public_post = create(:forum_post, body: "public", creator: @user) + end + end + + # should ignore the existence of private forum posts the current user doesn't have access to. + should respond_to_search(forum_posts: { body: "private" }).with { [] } + should respond_to_search(forum_posts: { body: "public" }).with { [@user] } + end end end