From e8efdd60d41c51bac8f2c4e2a514428018eb43b6 Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Mon, 17 Feb 2020 15:11:15 +1000 Subject: [PATCH] FIX: Tweak upload security emoji check (#8981) Further on from my earlier PR #8973 also reject upload as secure if its origin URL contains images/emoji. We still check Emoji.all first to try and be canonical. This may be a little heavy handed (e.g. if an external URL followed this same path it would be a false positive), but there are a lot of emoji aliases where the actual Emoji url is something, but you can have another image that should not be secure that that thing is an alias for. For example slight_smile.png does not show up in Emoji.all BUT slightly_smiling_face does, and it aliases slight_smile e.g. /images/emoji/twitter/slight_smile.png?v=9 and /images/emoji/twitter/slightly_smiling_face.png?v=9 are equivalent. --- lib/upload_security.rb | 3 ++- spec/models/upload_spec.rb | 9 +++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/lib/upload_security.rb b/lib/upload_security.rb index 79e162734b..039b03d158 100644 --- a/lib/upload_security.rb +++ b/lib/upload_security.rb @@ -78,6 +78,7 @@ class UploadSecurity def based_on_regular_emoji? return false if @upload.origin.blank? uri = URI.parse(@upload.origin) - Emoji.all.map(&:url).include?("#{uri.path}?#{uri.query}") + return true if Emoji.all.map(&:url).include?("#{uri.path}?#{uri.query}") + uri.path.include?("images/emoji") end end diff --git a/spec/models/upload_spec.rb b/spec/models/upload_spec.rb index 73ce096ba8..b2a1972152 100644 --- a/spec/models/upload_spec.rb +++ b/spec/models/upload_spec.rb @@ -438,6 +438,15 @@ describe Upload do upload.update!(secure: false, origin: "http://localhost:3000#{grinning.url}") expect { upload.update_secure_status } .not_to change { upload.secure } + expect(upload.reload.secure).to eq(false) + end + + it 'does not mark any upload with origin containing images/emoji in the URL' do + SiteSetting.login_required = true + upload.update!(secure: false, origin: "http://localhost:3000/images/emoji/test.png") + expect { upload.update_secure_status } + .not_to change { upload.secure } + expect(upload.reload.secure).to eq(false) end end end