Fix banned text being able to be circumvented via unicode (#35978)

app/lib/antispam.rb

@@ -32,26 +32,30 @@ class Antispam
     end
   end
 
-  def local_preflight_check!(status)
-    return unless considered_spam?(status)
+  def initialize(status)
+    @status = status
+  end
 
-    report_if_needed!(status.account)
+  def local_preflight_check!
+    return unless considered_spam?
 
-    raise SilentlyDrop, status
+    report_if_needed!
+
+    raise SilentlyDrop, @status
   end
 
   private
 
-  def considered_spam?(status)
-    (all_time_suspicious?(status) || recent_suspicious?(status)) && suspicious_reply_or_mention?(status)
+  def considered_spam?
+    (all_time_suspicious? || recent_suspicious?) && suspicious_reply_or_mention?
   end
 
-  def all_time_suspicious?(status)
-    all_time_spammy_texts.any? { |spammy_text| status.text.include?(spammy_text) }
+  def all_time_suspicious?
+    all_time_spammy_texts.any? { |spammy_text| status_text.include?(spammy_text) }
   end
 
-  def recent_suspicious?(status)
-    status.account.created_at >= ACCOUNT_AGE_EXEMPTION.ago && spammy_texts.any? { |spammy_text| status.text.include?(spammy_text) }
+  def recent_suspicious?
+    @status.account.created_at >= ACCOUNT_AGE_EXEMPTION.ago && spammy_texts.any? { |spammy_text| status_text.include?(spammy_text) }
   end
 
   def spammy_texts
@@ -62,25 +66,26 @@ class Antispam
     redis.smembers('antispam:all_time_spammy_texts')
   end
 
-  def suspicious_reply_or_mention?(status)
-    parent = status.thread
-    return true if parent.present? && !Follow.exists?(account_id: parent.account_id, target_account: status.account_id)
-
-    account_ids = status.mentions.map(&:account_id).uniq
-    !Follow.exists?(account_id: account_ids, target_account_id: status.account.id)
+  def suspicious_reply_or_mention?
+    account_ids = ([@status.in_reply_to_account_id] + @status.mentions.map(&:account_id)).uniq
+    !Follow.exists?(account_id: account_ids, target_account_id: @status.account.id)
   end
 
-  def report_if_needed!(account)
-    return if system_reports.unresolved.exists?(target_account: account)
+  def report_if_needed!
+    return if system_reports.unresolved.exists?(target_account: @status.account)
 
     system_reports.create!(
       category: :spam,
       comment: 'Account automatically reported for posting a banned URL',
-      target_account: account
+      target_account: @status.account
     )
   end
 
   def system_reports
     Account.representative.reports
   end
+
+  def status_text
+    @status_text ||= @status.text.unicode_normalize(:nfkc).downcase
+  end
 end

app/services/post_status_service.rb

@@ -39,8 +39,6 @@ class PostStatusService < BaseService
     @in_reply_to = @options[:thread]
     @quoted_status = @options[:quoted_status]
 
-    @antispam = Antispam.new
-
     return idempotency_duplicate if idempotency_given? && idempotency_duplicate?
 
     validate_media!
@@ -82,7 +80,9 @@ class PostStatusService < BaseService
     process_mentions_service.call(@status, save_records: false)
     safeguard_mentions!(@status)
     attach_quote!(@status)
-    @antispam.local_preflight_check!(@status)
+
+    antispam = Antispam.new(@status)
+    antispam.local_preflight_check!
 
     # The following transaction block is needed to wrap the UPDATEs to
     # the media attachments when the status is created
@@ -113,7 +113,9 @@ class PostStatusService < BaseService
 
   def schedule_status!
     status_for_validation = @account.statuses.build(status_attributes)
-    @antispam.local_preflight_check!(status_for_validation)
+
+    antispam = Antispam.new(status_for_validation)
+    antispam.local_preflight_check!
 
     if status_for_validation.valid?
       # Marking the status as destroyed is necessary to prevent the status from being

spec/lib/antispam_spec.rb

@@ -4,7 +4,7 @@ require 'rails_helper'
 
 RSpec.describe Antispam do
   describe '#local_preflight_check!' do
-    subject { described_class.new.local_preflight_check!(status) }
+    subject { described_class.new(status).local_preflight_check! }
 
     let(:status) { Fabricate :status }
 
@@ -39,6 +39,30 @@ RSpec.describe Antispam do
         end
       end
 
+      context 'when status matches unicode variants' do
+        let(:status) { Fabricate :status, text: 'I use https://𝐛𝐚𝐧𝐧𝐞𝐝.𝐞𝐱𝐚𝐦𝐩𝐥𝐞 urls in my text' }
+
+        it 'raises error and reports' do
+          expect { subject }
+            .to raise_error(described_class::SilentlyDrop)
+            .and change(spam_reports, :count).by(1)
+        end
+
+        context 'when report already exists' do
+          before { Fabricate :report, account: Account.representative, target_account: status.account }
+
+          it 'raises error and does not report' do
+            expect { subject }
+              .to raise_error(described_class::SilentlyDrop)
+              .and not_change(spam_reports, :count)
+          end
+        end
+
+        def spam_reports
+          Account.representative.reports.where(target_account: status.account).spam
+        end
+      end
+
       context 'when status does not match' do
         it { is_expected.to be_nil }
       end