From 20e97f8d8d7cf93991686f9ea640a1bce06f4f9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?KMY=EF=BC=88=E9=9B=AA=E3=81=82=E3=81=99=E3=81=8B=EF=BC=89?= Date: Thu, 26 Oct 2023 08:22:24 +0900 Subject: [PATCH] Merge pull request from GHSA-c7p6-c688-fhgp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * FixSec: スタンプを経由してカスタム絵文字のデータを改竄できる問題 * Fix: 相乗り絵文字が受け取れない問題とテスト * Change: 相乗りスタンプは常にオリジナルの情報を参照 * Fix: uriまで偽装した場合に対応 --- app/lib/activitypub/activity/like.rb | 17 ++- .../activitypub/emoji_serializer.rb | 6 +- spec/lib/activitypub/activity/like_spec.rb | 114 +++++++++++++++++- 3 files changed, 127 insertions(+), 10 deletions(-) diff --git a/app/lib/activitypub/activity/like.rb b/app/lib/activitypub/activity/like.rb index 8be83456139eae..729a1ba0c6a4ca 100644 --- a/app/lib/activitypub/activity/like.rb +++ b/app/lib/activitypub/activity/like.rb @@ -3,6 +3,7 @@ class ActivityPub::Activity::Like < ActivityPub::Activity include Redisable include Lockable + include JsonLdHelper def perform @original_status = status_from_uri(object_uri) @@ -102,7 +103,7 @@ def process_emoji(tag) return if custom_emoji_parser.shortcode.blank? || custom_emoji_parser.image_remote_url.blank? - domain = tag['domain'] || URI.split(custom_emoji_parser.uri)[2] || @account.domain + domain = URI.split(custom_emoji_parser.uri)[2] || @account.domain if domain == Rails.configuration.x.local_domain || domain == Rails.configuration.x.web_domain # Block overwriting remote-but-local data @@ -118,6 +119,9 @@ def process_emoji(tag) (custom_emoji_parser.updated_at && custom_emoji_parser.updated_at >= emoji.updated_at) || custom_emoji_parser.license != emoji.license + custom_emoji_parser = original_emoji_parser(custom_emoji_parser) if @account.domain != domain + return if custom_emoji_parser.nil? + begin emoji ||= CustomEmoji.new( domain: domain, @@ -136,6 +140,17 @@ def process_emoji(tag) emoji end + def original_emoji_parser(custom_emoji_parser) + uri = custom_emoji_parser.uri + emoji = fetch_resource_without_id_validation(uri) + return nil unless emoji + + parser = ActivityPub::Parser::CustomEmojiParser.new(emoji) + return nil unless parser.uri == uri && custom_emoji_parser.shortcode == parser.shortcode + + parser + end + def skip_download?(domain) DomainBlock.reject_media?(domain) end diff --git a/app/serializers/activitypub/emoji_serializer.rb b/app/serializers/activitypub/emoji_serializer.rb index a8dfff58f6f84c..24c21c3ccf2df5 100644 --- a/app/serializers/activitypub/emoji_serializer.rb +++ b/app/serializers/activitypub/emoji_serializer.rb @@ -5,7 +5,7 @@ class ActivityPub::EmojiSerializer < ActivityPub::Serializer context_extensions :emoji, :license, :keywords - attributes :id, :type, :domain, :name, :keywords, :is_sensitive, :updated + attributes :id, :type, :name, :keywords, :is_sensitive, :updated attribute :license, if: -> { object.license.present? } @@ -19,10 +19,6 @@ def type 'Emoji' end - def domain - object.domain.presence || Rails.configuration.x.local_domain - end - def keywords object.aliases end diff --git a/spec/lib/activitypub/activity/like_spec.rb b/spec/lib/activitypub/activity/like_spec.rb index 25037ed6355392..804d2410522fde 100644 --- a/spec/lib/activitypub/activity/like_spec.rb +++ b/spec/lib/activitypub/activity/like_spec.rb @@ -16,6 +16,28 @@ object: ActivityPub::TagManager.instance.uri_for(status), }.with_indifferent_access end + let(:original_emoji) do + { + id: 'https://example.com/aaa', + type: 'Emoji', + icon: { + url: 'http://example.com/emoji.png', + }, + name: 'tinking', + license: 'This is ohagi', + } + end + let(:original_invalid_emoji) do + { + id: 'https://example.com/invalid', + type: 'Emoji', + icon: { + url: 'http://example.com/emoji.png', + }, + name: 'other', + license: 'This is other ohagi', + } + end describe '#perform' do subject { described_class.new(json, sender) } @@ -37,6 +59,9 @@ before do stub_request(:get, 'http://example.com/emoji.png').to_return(body: attachment_fixture('emojo.png')) + stub_request(:get, 'http://foo.bar/emoji2.png').to_return(body: attachment_fixture('emojo.png')) + stub_request(:get, 'https://example.com/aaa').to_return(status: 200, body: Oj.dump(original_emoji)) + stub_request(:get, 'https://example.com/invalid').to_return(status: 200, body: Oj.dump(original_invalid_emoji)) end let(:json) do @@ -122,13 +147,12 @@ end end - context 'with custom emoji and custom domain' do + context 'with custom emoji from non-original server account' do let(:content) { ':tinking:' } let(:tag) do { id: 'https://example.com/aaa', type: 'Emoji', - domain: 'post.kmycode.net', icon: { url: 'http://example.com/emoji.png', }, @@ -136,17 +160,98 @@ } end + before do + sender.update(domain: 'ohagi.com') + Fabricate(:custom_emoji, domain: 'example.com', uri: 'https://example.com/aaa', shortcode: 'tinking') + end + it 'create emoji reaction' do expect(subject.count).to eq 1 expect(subject.first.name).to eq 'tinking' expect(subject.first.account).to eq sender expect(subject.first.custom_emoji).to_not be_nil expect(subject.first.custom_emoji.shortcode).to eq 'tinking' - expect(subject.first.custom_emoji.domain).to eq 'post.kmycode.net' + expect(subject.first.custom_emoji.domain).to eq 'example.com' + expect(sender.favourited?(status)).to be false + end + end + + context 'with custom emoji and update license from non-original server account' do + let(:content) { ':tinking:' } + let(:tag) do + { + id: 'https://example.com/aaa', + type: 'Emoji', + icon: { + url: 'http://example.com/emoji.png', + }, + name: 'tinking', + license: 'Old license', + } + end + + before do + sender.update(domain: 'ohagi.com') + Fabricate(:custom_emoji, domain: 'example.com', uri: 'https://example.com/aaa', shortcode: 'tinking') + end + + it 'create emoji reaction' do + expect(subject.count).to eq 1 + expect(subject.first.custom_emoji.license).to eq 'This is ohagi' expect(sender.favourited?(status)).to be false end end + context 'with custom emoji but icon url is not valid' do + let(:content) { ':tinking:' } + let(:tag) do + { + id: 'https://example.com/aaa', + type: 'Emoji', + icon: { + url: 'http://foo.bar/emoji.png', + }, + name: 'tinking', + license: 'Good for using darwin', + } + end + + before do + sender.update(domain: 'ohagi.com') + Fabricate(:custom_emoji, domain: 'example.com', uri: 'https://example.com/aaa', shortcode: 'tinking', image_remote_url: 'http://example.com/emoji.png') + end + + it 'create emoji reaction' do + expect(subject.count).to eq 1 + expect(subject.first.custom_emoji.reload.license).to eq 'This is ohagi' + expect(subject.first.custom_emoji.image_remote_url).to eq 'http://example.com/emoji.png' + end + end + + context 'with custom emoji but uri is not valid' do + let(:content) { ':tinking:' } + let(:tag) do + { + id: 'https://example.com/invalid', + type: 'Emoji', + icon: { + url: 'http://foo.bar/emoji2.png', + }, + name: 'tinking', + license: 'Good for using darwin', + } + end + + before do + sender.update(domain: 'ohagi.com') + Fabricate(:custom_emoji, domain: 'example.com', uri: 'https://example.com/aaa', shortcode: 'tinking', image_remote_url: 'http://example.com/emoji.png') + end + + it 'create emoji reaction' do + expect(subject.count).to eq 0 + end + end + context 'with custom emoji but invalid id' do let(:content) { ':tinking:' } let(:tag) do @@ -175,7 +280,7 @@ let(:content) { ':tinking:' } let(:tag) do { - id: 'aaa', + id: 'https://cb6e6126.ngrok.io/aaa', type: 'Emoji', domain: Rails.configuration.x.local_domain, icon: { @@ -197,6 +302,7 @@ expect(subject.first.custom_emoji).to_not be_nil expect(subject.first.custom_emoji.shortcode).to eq 'tinking' expect(subject.first.custom_emoji.domain).to be_nil + expect(subject.first.custom_emoji.license).to eq 'Everyone but Ohagi' expect(sender.favourited?(status)).to be false end