From dc55e9cdf93a4c61bfb97d0a0bb1d44137d38422 Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Tue, 24 Jan 2023 10:01:48 +1000 Subject: [PATCH] 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. --- lib/upload_security.rb | 10 +++++++++- spec/lib/upload_security_spec.rb | 10 ++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/lib/upload_security.rb b/lib/upload_security.rb index c5de6c17bff..c00c1376ecf 100644 --- a/lib/upload_security.rb +++ b/lib/upload_security.rb @@ -136,7 +136,15 @@ class UploadSecurity def publicly_referenced_first_check 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? PUBLIC_UPLOAD_REFERENCE_TYPES.include?(first_reference.target_type) end diff --git a/spec/lib/upload_security_spec.rb b/spec/lib/upload_security_spec.rb index 1a70bfecfa1..8834ff4fc40 100644 --- a/spec/lib/upload_security_spec.rb +++ b/spec/lib/upload_security_spec.rb @@ -217,6 +217,16 @@ RSpec.describe UploadSecurity do 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 it "returns false" do SiteSetting.favicon = upload