FIX: Do not count deleted post for upload ref security (#19949)

When checking whether an existing upload should be secure
based on upload references, do not count deleted posts, since
there is still a reference attached to them. This can lead to
issues where e.g. an upload is used for a post then later on
a custom emoji.
This commit is contained in:
Martin Brennan 2023-01-24 10:01:48 +10:00 committed by Bianca Nenciu
parent 7df88e338a
commit dc55e9cdf9
2 changed files with 19 additions and 1 deletions

View File

@ -136,7 +136,15 @@ class UploadSecurity
def publicly_referenced_first_check def publicly_referenced_first_check
return false if @creating return false if @creating
first_reference = @upload.upload_references.order(created_at: :asc).first first_reference =
@upload
.upload_references
.joins(<<~SQL)
LEFT JOIN posts ON upload_references.target_type = 'Post' AND upload_references.target_id = posts.id
SQL
.where("posts.deleted_at IS NULL")
.order(created_at: :asc)
.first
return false if first_reference.blank? return false if first_reference.blank?
PUBLIC_UPLOAD_REFERENCE_TYPES.include?(first_reference.target_type) PUBLIC_UPLOAD_REFERENCE_TYPES.include?(first_reference.target_type)
end end

View File

@ -217,6 +217,16 @@ RSpec.describe UploadSecurity do
end end
end end
describe "when the upload is first used for a post in a secure context that is later deleted" do
it "returns false" do
create_secure_post_reference
post_in_secure_context.trash!
CustomEmoji.create(name: "meme", upload: upload)
expect(subject.should_be_secure?).to eq(false)
end
end
describe "when the upload is first used for a site setting" do describe "when the upload is first used for a site setting" do
it "returns false" do it "returns false" do
SiteSetting.favicon = upload SiteSetting.favicon = upload