Skip to content

Commit

Permalink
SECURITY: prevent topic list filtering by hidden tags for unathorized…
Browse files Browse the repository at this point in the history
… users

This fixes an issue where unathorized users were able to filter topics
by tags that are hidden from them.
  • Loading branch information
pmusaraj authored and tgxworld committed Oct 7, 2024
1 parent f08cd7f commit 2506257
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 1 deletion.
4 changes: 3 additions & 1 deletion lib/topic_query.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1281,7 +1281,9 @@ def filter_by_tags(result)

if tags_arg && tags_arg.size > 0
tags_arg = tags_arg.split if String === tags_arg
tags_query = tags_arg[0].is_a?(String) ? Tag.where_name(tags_arg) : Tag.where(id: tags_arg)
tags_query = DiscourseTagging.visible_tags(@guardian)
tags_query =
tags_arg[0].is_a?(String) ? tags_query.where_name(tags_arg) : tags_query.where(id: tags_arg)
tags = tags_query.select(:id, :target_tag_id).map { |t| t.target_tag_id || t.id }.uniq

if ActiveModel::Type::Boolean.new.cast(@options[:match_all_tags])
Expand Down
40 changes: 40 additions & 0 deletions spec/lib/topic_query_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -572,6 +572,46 @@
tagged_topic3,
)
end

context "with hidden tags" do
let(:hidden_tag) { Fabricate(:tag, name: "hidden") }
let!(:staff_tag_group) do
Fabricate(:tag_group, permissions: { "staff" => 1 }, tag_names: [hidden_tag.name])
end
let!(:topic_with_hidden_tag) { Fabricate(:topic, tags: [tag, hidden_tag]) }

it "returns topics with hidden tag to admin" do
expect(
TopicQuery.new(admin, tags: hidden_tag.name).list_latest.topics,
).to contain_exactly(topic_with_hidden_tag)
end

it "doesn't return topics with hidden tags to anon" do
expect(TopicQuery.new(nil, tags: hidden_tag.name).list_latest.topics).to be_empty
end

it "doesn't return topic with hidden tags to non-staff" do
expect(TopicQuery.new(user, tags: hidden_tag.name).list_latest.topics).to be_empty
end

it "returns topics with hidden tag to admin when using match_all_tags" do
expect(
TopicQuery
.new(admin, tags: [tag.name, hidden_tag.name], match_all_tags: true)
.list_latest
.topics,
).to contain_exactly(topic_with_hidden_tag)
end

it "doesn't return topic with hidden tags to non-staff when using match_all_tags" do
expect(
TopicQuery
.new(user, tags: [tag.name, hidden_tag.name], match_all_tags: true)
.list_latest
.topics,
).to be_empty
end
end
end

context "when remove_muted_tags is enabled" do
Expand Down

0 comments on commit 2506257

Please sign in to comment.