Skip to content

Commit

Permalink
Merge pull request from GHSA-c7p6-c688-fhgp
Browse files Browse the repository at this point in the history
* FixSec: スタンプを経由してカスタム絵文字のデータを改竄できる問題

* Fix: 相乗り絵文字が受け取れない問題とテスト

* Change: 相乗りスタンプは常にオリジナルの情報を参照

* Fix: uriまで偽装した場合に対応
  • Loading branch information
kmycode authored Oct 25, 2023
1 parent 1218434 commit 20e97f8
Show file tree
Hide file tree
Showing 3 changed files with 127 additions and 10 deletions.
17 changes: 16 additions & 1 deletion app/lib/activitypub/activity/like.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand All @@ -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
Expand Down
6 changes: 1 addition & 5 deletions app/serializers/activitypub/emoji_serializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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? }

Expand All @@ -19,10 +19,6 @@ def type
'Emoji'
end

def domain
object.domain.presence || Rails.configuration.x.local_domain
end

def keywords
object.aliases
end
Expand Down
114 changes: 110 additions & 4 deletions spec/lib/activitypub/activity/like_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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) }
Expand All @@ -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
Expand Down Expand Up @@ -122,31 +147,111 @@
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',
},
name: 'tinking',
}
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
Expand Down Expand Up @@ -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: {
Expand All @@ -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

Expand Down

0 comments on commit 20e97f8

Please sign in to comment.