ActivityPub.inbox: start on same-domain check for actors and activity…

…/object ids

for GHSA-37r7-jqmr-3472

activitypub.py

@@ -37,6 +37,7 @@
     PRIMARY_DOMAIN,
     PROTOCOL_DOMAINS,
     redirect_wrap,
+    report_error,
     subdomain_wrap,
     unwrap,
 )
@@ -1070,31 +1071,19 @@ def inbox(protocol=None, id=None):
 
     # do we support this object type?
     # (this logic is duplicated in Protocol.check_supported)
-    object = as1.get_object(activity)
+    obj = as1.get_object(activity)
     if type := activity.get('type'):
-        inner_type = as1.object_type(object) or ''
+        inner_type = as1.object_type(obj) or ''
         if (type not in ActivityPub.SUPPORTED_AS2_TYPES or
             (type in as2.CRUD_VERBS
              and inner_type
              and inner_type not in ActivityPub.SUPPORTED_AS2_TYPES)):
             error(f"Bridgy Fed for ActivityPub doesn't support {type} {inner_type} yet: {json_dumps(activity, indent=2)}", status=204)
 
-    # are we already processing or done with this activity?
-    id = activity.get('id')
-    if id:
-        domain = util.domain_from_link(id)
-        if util.domain_or_parent_in(domain, web_opt_out_domains()):
-            logger.info(f'{domain} is opted out')
-            return '', 204
-
-        if memcache.get(activity_id_memcache_key(id)):
-            logger.info(f'Already seen {id}')
-            return '', 204
-
-    # check actor, signature, auth
+    # check actor, authz actor's domain against activity and object ids
+    # https://github.com/snarfed/bridgy-fed/security/advisories/GHSA-37r7-jqmr-3472
     actor = as1.get_object(activity, 'actor')
     actor_id = actor.get('id')
-    logger.info(f'Got {type} {id} from {actor_id}')
 
     if ActivityPub.is_blocklisted(actor_id):
         error(f'Actor {actor_id} is blocklisted')
@@ -1104,6 +1093,28 @@ def inbox(protocol=None, id=None):
         logger.info(f'{actor_domain} is opted out')
         return '', 204
 
+    id = activity.get('id')
+    obj_id = obj.get('id')
+    if id and actor_domain != util.domain_from_link(id):
+        report_error('Auth: actor and activity on different domains',
+                     user=f'actor {actor_id} activity {id}')
+    elif (type in as2.CRUD_VERBS and obj_id
+          and actor_domain != util.domain_from_link(obj_id)):
+        report_error('Auth: actor and object on different domains',
+                     user=f'actor {actor_id} object {id}')
+
+    # are we already processing or done with this activity?
+    if id:
+        domain = util.domain_from_link(id)
+        if util.domain_or_parent_in(domain, web_opt_out_domains()):
+            logger.info(f'{domain} is opted out')
+            return '', 204
+
+        if memcache.get(activity_id_memcache_key(id)):
+            logger.info(f'Already seen {id}')
+            return '', 204
+
+    # check signature, auth
     authed_as = ActivityPub.verify_signature(activity)
 
     authed_domain = util.domain_from_link(authed_as)
@@ -1115,6 +1126,8 @@ def inbox(protocol=None, id=None):
     if authed_as != actor_id and activity.get('signature'):
         error(f"Ignoring LD Signature, sorry, we can't verify those yet. https://github.com/snarfed/bridgy-fed/issues/566", status=202)
 
+    logger.info(f'Got {type} {id} from {actor_id}')
+
     if type == 'Follow':
         # rendered mf2 HTML proxy pages (in render.py) fall back to redirecting
         # to the follow's AS2 id field, but Mastodon's Accept ids are URLs that
@@ -1128,7 +1141,7 @@ def inbox(protocol=None, id=None):
         activity.setdefault('url', f'{follower_url}#followed-{followee_url}')
 
     if not id:
-        id = f'{actor_id}#{type}-{object.get("id", "")}-{util.now().isoformat()}'
+        id = f'{actor_id}#{type}-{obj_id or ""}-{util.now().isoformat()}'
 
     # automatically bridge server aka instance actors
     # https://codeberg.org/fediverse/fep/src/branch/main/fep/d556/fep-d556.md

tests/test_activitypub.py

@@ -1696,13 +1696,9 @@ def test_delete_actor(self, *mocks):
     def test_delete_actor_not_fetchable(self, _, mock_get, ___):
         mock_get.return_value = requests_response(status=410)
 
-        with self.assertLogs() as logs:
-            got = self.post('/ap/sharedInbox', json=DELETE)
-
+        got = self.post('/ap/sharedInbox', json=DELETE)
         self.assertEqual(202, got.status_code)
         self.assertTrue(Object.get_by_id(DELETE['object']).deleted)
-        # self.assertIn('Object/actor being deleted is also keyId',
-        #               ' '.join(logs.output))
 
     def test_delete_actor_empty_deleted_object(self, _, mock_get, ___):
         actor = self.make_user(DELETE['actor'], cls=ActivityPub)
@@ -1880,6 +1876,50 @@ def test_inbox_existing_server_actor_adds_enabled_protocols(
         actor = ActivityPub.get_by_id('https://mas.to/actor')
         self.assertCountEqual(['ui', 'fake', 'other'], actor.enabled_protocols)
 
+    # https://github.com/snarfed/bridgy-fed/security/advisories/GHSA-37r7-jqmr-3472
+    def test_inbox_actor_auth_check_activity_id_different_domain(
+            self, mock_head, mock_get, mock_post):
+        mock_get.side_effect = [
+            self.as2_resp(ACTOR),
+            self.as2_resp(ACTOR),
+            self.as2_resp(NOTE),
+        ]
+
+        with self.assertLogs() as logs:
+            got = self.post('/user.com/inbox', json={
+                'id': 'http://no.pe/like',
+                'type': 'Like',
+                'actor': 'https://mas.to/users/swentel',
+                'object': 'https://mas.to/note/as2',
+            })
+
+        self.assertEqual(204, got.status_code)
+        self.assertIn('Auth: actor and activity on different domains',
+                      ' '.join(logs.output))
+
+    # https://github.com/snarfed/bridgy-fed/security/advisories/GHSA-37r7-jqmr-3472
+    def test_inbox_actor_auth_check_object_id_different_domain(
+            self, mock_head, mock_get, mock_post):
+        mock_get.side_effect = [
+            self.as2_resp(ACTOR),
+            self.as2_resp(ACTOR),
+        ]
+
+        with self.assertLogs() as logs:
+            got = self.post('/user.com/inbox', json={
+                'id': 'http://mas.to/create',
+                'type': 'Create',
+                'actor': 'https://mas.to/users/swentel',
+                'object': {
+                    **NOTE_OBJECT,
+                    'id': 'https://no.pe/note',
+                },
+            })
+
+        self.assertEqual(204, got.status_code)
+        self.assertIn('Auth: actor and object on different domains',
+                      ' '.join(logs.output))
+
     def test_followers_collection_unknown_user(self, *_):
         resp = self.client.get('/nope.com/followers')
         self.assertEqual(404, resp.status_code)