diff --git a/CHANGELOG.md b/CHANGELOG.md index c4a6fbc..615d045 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Unreleased + +### Fixed + +* Address CVE-2024-23832 by ensuring that a pulled AP payload id netloc is the same as the request fid netloc. + ## [0.25.0] - 2024-01-06 ### Added diff --git a/federation/tests/utils/test_activitypub.py b/federation/tests/utils/test_activitypub.py index 46e7d46..3500152 100644 --- a/federation/tests/utils/test_activitypub.py +++ b/federation/tests/utils/test_activitypub.py @@ -57,7 +57,8 @@ class TestRetrieveAndParseDocument: ) @patch.object(Follow, "post_receive") def test_returns_entity_for_valid_document__follow(self, mock_post_receive, mock_fetch, mock_recv): - entity = retrieve_and_parse_document("https://example.com/foobar") + entity = retrieve_and_parse_document("https://example.com/follow") + print(entity) assert isinstance(entity, Follow) @patch("federation.entities.activitypub.models.get_profile_or_entity", return_value=None) @@ -65,7 +66,7 @@ class TestRetrieveAndParseDocument: json.dumps(ACTIVITYPUB_POST_OBJECT), None, None), ) def test_returns_entity_for_valid_document__post__without_activity(self, mock_fetch, mock_recv): - entity = retrieve_and_parse_document("https://example.com/foobar") + entity = retrieve_and_parse_document("https://diaspodon.fr/users/jaywink/statuses/102356911717767237") assert isinstance(entity, Note) @patch("federation.entities.activitypub.models.extract_receivers", return_value=[]) @@ -73,7 +74,7 @@ class TestRetrieveAndParseDocument: json.dumps(ACTIVITYPUB_POST_OBJECT_IMAGES), None, None), ) def test_returns_entity_for_valid_document__post__without_activity__with_images(self, mock_fetch, mock_recv): - entity = retrieve_and_parse_document("https://example.com/foobar") + entity = retrieve_and_parse_document("https://mastodon.social/users/foobar/statuses/34324r") assert isinstance(entity, Note) assert len(entity._children) == 1 assert entity._children[0].url == "https://files.mastodon.social/media_attachments/files/017/792/237/original" \ @@ -86,9 +87,19 @@ class TestRetrieveAndParseDocument: ) def test_returns_entity_for_valid_document__post__wrapped_in_activity( self, mock_fetch, mock_recv, mock_sign): - entity = retrieve_and_parse_document("https://example.com/foobar") + entity = retrieve_and_parse_document("https://diaspodon.fr/users/jaywink/statuses/102356911717767237/activity") assert isinstance(entity, Note) + @patch("federation.entities.activitypub.models.verify_ld_signature", return_value=None) + @patch("federation.entities.activitypub.models.get_profile_or_entity", return_value=None) + @patch("federation.utils.activitypub.fetch_document", autospec=True, return_value=( + json.dumps(ACTIVITYPUB_POST), None, None), + ) + def test_returns_none_for_forged_document( + self, mock_fetch, mock_recv, mock_sign): + entity = retrieve_and_parse_document("https://example.com/foobar") + assert entity is None + @patch("federation.utils.activitypub.fetch_document", autospec=True, return_value=('{"foo": "bar"}', None, None)) def test_returns_none_for_invalid_document(self, mock_fetch): entity = retrieve_and_parse_document("https://example.com/foobar") diff --git a/federation/utils/activitypub.py b/federation/utils/activitypub.py index 62404cc..55b462e 100644 --- a/federation/utils/activitypub.py +++ b/federation/utils/activitypub.py @@ -1,6 +1,7 @@ import json import logging from typing import Optional, Any +from urllib.parse import urlparse from federation.protocols.activitypub.signing import get_http_authentication from federation.utils.network import fetch_document, try_retrieve_webfinger_document @@ -52,8 +53,14 @@ def retrieve_and_parse_document(fid: str, cache: bool=True) -> Optional[Any]: return None entities = element_to_objects(document) if entities: - logger.info("retrieve_and_parse_document - using first entity: %s", entities[0]) - return entities[0] + entity = entities[0] + id = entity.id or entity.activity_id + # check against potential payload forgery (CVE-2024-23832) + if urlparse(id).netloc != urlparse(fid).netloc: + logger.warning('retrieve_and_parse_document - payload may be forged, discarding: %s', fid) + return None + logger.info("retrieve_and_parse_document - using first entity: %s", entity) + return entity def retrieve_and_parse_profile(fid: str) -> Optional[Any]: