Fix #5237: Deleted comments can be viewed by other users

* Fix it so non-moderators can't search deleted comments using the
  `updater`, `body`, `score`, `do_not_bump_post`, or `is_sticky` fields.
  Searching for these fields will exclude deleted comments.

* Fix it so non-moderators can search for their own deleted comments using the
  `creator` field, but not for deleted comments belonging to other users.

* Fix it so that if a regular user searches `commenter:<username>`, they
  can only see posts with undeleted comments by that user. If a moderator or
  the commenter themselves searches `commenter:<username>`, they can see all
  posts the user has commented on, including posts with deleted comments.

* Fix it so the comment count on user profiles only counts visible
  comments. Regular users can only see the number of undeleted comments
  a user has, while moderators and the commenter themselves can see the
  total number of comments.

Known issue:

* It's still possible to order deleted comments by score, which can let
  you infer the score of deleted comments.
This commit is contained in:
evazion
2022-09-22 19:02:17 -05:00
parent 88ac91f5f3
commit a442658f8a
8 changed files with 88 additions and 23 deletions

View File

@@ -138,7 +138,7 @@ class PostQuery
# True if the search depends on the current user because of permissions or privacy settings.
def is_user_dependent_search?
metatags.any? do |metatag|
metatag.name.in?(%w[upvoter upvote downvoter downvote search flagger fav ordfav favgroup ordfavgroup]) ||
metatag.name.in?(%w[upvoter upvote downvoter downvote commenter comm search flagger fav ordfav favgroup ordfavgroup]) ||
metatag.name == "status" && metatag.value == "unmoderated" ||
metatag.name == "disapproved" && !metatag.value.downcase.in?(PostDisapproval::REASONS)
end

View File

@@ -172,19 +172,19 @@ class PostQueryBuilder
when "flagger"
relation.flagger_matches(value, current_user)
when "appealer"
relation.user_subquery_matches(PostAppeal.unscoped, value)
relation.user_subquery_matches(PostAppeal.unscoped, value, current_user)
when "commenter", "comm"
relation.user_subquery_matches(Comment.unscoped, value)
relation.user_subquery_matches(Comment.unscoped, value, current_user)
when "commentaryupdater", "artcomm"
relation.user_subquery_matches(ArtistCommentaryVersion.unscoped, value, field: :updater)
relation.user_subquery_matches(ArtistCommentaryVersion.unscoped, value, current_user, field: :updater)
when "noter"
relation.user_subquery_matches(NoteVersion.unscoped.where(version: 1), value, field: :updater)
relation.user_subquery_matches(NoteVersion.unscoped.where(version: 1), value, current_user, field: :updater)
when "noteupdater"
relation.user_subquery_matches(NoteVersion.unscoped, value, field: :updater)
relation.user_subquery_matches(NoteVersion.unscoped, value, current_user, field: :updater)
when "upvoter", "upvote"
relation.user_subquery_matches(PostVote.active.positive.visible(current_user), value, field: :user)
relation.user_subquery_matches(PostVote.active.positive.visible(current_user), value, current_user, field: :user)
when "downvoter", "downvote"
relation.user_subquery_matches(PostVote.active.negative.visible(current_user), value, field: :user)
relation.user_subquery_matches(PostVote.active.negative.visible(current_user), value, current_user, field: :user)
when "random"
relation # handled in the `build` method
when *CATEGORY_COUNT_METATAGS

View File

@@ -44,6 +44,10 @@ class ApplicationRecord < ActiveRecord::Base
all
end
def visible_for_search(attribute, current_user)
policy(current_user).visible_for_search(all, attribute)
end
def policy(current_user)
Pundit.policy(current_user, self)
end

View File

@@ -1359,7 +1359,7 @@ class Post < ApplicationRecord
end
end
def user_subquery_matches(subquery, username, field: :creator, &block)
def user_subquery_matches(subquery, username, current_user, field: :creator, &block)
subquery = subquery.where("post_id = posts.id").select(1)
if username.downcase == "any"
@@ -1369,7 +1369,7 @@ class Post < ApplicationRecord
elsif block.nil?
user = User.find_by_name(username)
return none if user.nil?
subquery = subquery.where(field => user)
subquery = subquery.visible_for_search(field, current_user).where(field => user)
where("EXISTS (#{subquery.to_sql})")
else
subquery = subquery.merge(block.call(username))

View File

@@ -561,7 +561,7 @@ class User < ApplicationRecord
end
def comment_count
comments.count
comments.visible_for_search(:creator, CurrentUser.user).count
end
def favorite_group_count

View File

@@ -43,5 +43,16 @@ class CommentPolicy < ApplicationPolicy
attributes
end
def visible_for_search(comments, attribute)
case attribute
in :creator | :creator_id if !can_see_deleted?
comments.where(creator: user, is_deleted: true).or(comments.undeleted)
in :updater | :updater_id | :body | :score | :do_not_bump_post | :is_sticky if !can_see_deleted?
comments.undeleted
else
comments
end
end
alias_method :undelete?, :update?
end