From 3a0c1dca901205efa8b01b4ee1e8c0a5ec172f77 Mon Sep 17 00:00:00 2001 From: Nutomic Date: Thu, 11 Apr 2024 16:05:49 +0200 Subject: [PATCH] Avoid overwriting local objects via federation (#4611) * Dont allow federation to overwrite local objects * is_local check in apub lib * use imports * fix check, update lib * use verify_is_remote_object() * submodule --- Cargo.lock | 4 ++-- Cargo.toml | 2 +- crates/apub/src/objects/comment.rs | 2 +- crates/apub/src/objects/instance.rs | 2 ++ crates/apub/src/objects/mod.rs | 25 ++++++++++++++++------ crates/apub/src/objects/person.rs | 2 ++ crates/apub/src/objects/post.rs | 2 +- crates/apub/src/objects/private_message.rs | 2 ++ crates/apub/src/protocol/objects/group.rs | 6 ++++-- 9 files changed, 34 insertions(+), 13 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6cf47cdd8..97504600b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -16,9 +16,9 @@ checksum = "8f27d075294830fcab6f66e320dab524bc6d048f4a151698e153205559113772" [[package]] name = "activitypub_federation" -version = "0.5.3" +version = "0.5.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7f2a21d597eb09353bc83bea36095e1de0ef5297a6fe16edba3de676898a5ba9" +checksum = "6e16130d5914e6483f99bde5a9bb97ca62e1f359e0b9791c8ebd5c7abd50fe8e" dependencies = [ "activitystreams-kinds", "actix-web", diff --git a/Cargo.toml b/Cargo.toml index 3ecba6d04..4d9485ca5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -99,7 +99,7 @@ lemmy_db_views = { version = "=0.19.4-beta.3", path = "./crates/db_views" } lemmy_db_views_actor = { version = "=0.19.4-beta.3", path = "./crates/db_views_actor" } lemmy_db_views_moderator = { version = "=0.19.4-beta.3", path = "./crates/db_views_moderator" } lemmy_federate = { version = "=0.19.4-beta.3", path = "./crates/federate" } -activitypub_federation = { version = "0.5.3", default-features = false, features = [ +activitypub_federation = { version = "0.5.4", default-features = false, features = [ "actix-web", ] } diesel = "2.1.4" diff --git a/crates/apub/src/objects/comment.rs b/crates/apub/src/objects/comment.rs index e3128e439..ce0430cc0 100644 --- a/crates/apub/src/objects/comment.rs +++ b/crates/apub/src/objects/comment.rs @@ -140,7 +140,7 @@ impl Object for ApubComment { let community = note.community(context).await?; check_apub_id_valid_with_strictness(note.id.inner(), community.local, context).await?; - verify_is_remote_object(note.id.inner(), context.settings())?; + verify_is_remote_object(¬e.id, context)?; verify_person_in_community(¬e.attributed_to, &community, context).await?; let (post, _) = note.get_parents(context).await?; diff --git a/crates/apub/src/objects/instance.rs b/crates/apub/src/objects/instance.rs index 021d2b1cd..145dc63c2 100644 --- a/crates/apub/src/objects/instance.rs +++ b/crates/apub/src/objects/instance.rs @@ -1,3 +1,4 @@ +use super::verify_is_remote_object; use crate::{ activities::GetActorType, check_apub_id_valid_with_strictness, @@ -124,6 +125,7 @@ impl Object for ApubSite { ) -> LemmyResult<()> { check_apub_id_valid_with_strictness(apub.id.inner(), true, data).await?; verify_domains_match(expected_domain, apub.id.inner())?; + verify_is_remote_object(&apub.id, data)?; let local_site_data = local_site_data_cached(&mut data.pool()).await?; let slur_regex = &local_site_opt_to_slur_regex(&local_site_data.local_site); diff --git a/crates/apub/src/objects/mod.rs b/crates/apub/src/objects/mod.rs index 693521ee1..e199ebfad 100644 --- a/crates/apub/src/objects/mod.rs +++ b/crates/apub/src/objects/mod.rs @@ -1,9 +1,16 @@ use crate::protocol::Source; -use activitypub_federation::protocol::values::MediaTypeMarkdownOrHtml; +use activitypub_federation::{ + config::Data, + fetch::object_id::ObjectId, + protocol::values::MediaTypeMarkdownOrHtml, + traits::Object, +}; use anyhow::anyhow; use html2md::parse_html; -use lemmy_utils::{error::LemmyResult, settings::structs::Settings}; -use url::Url; +use lemmy_api_common::context::LemmyContext; +use lemmy_utils::error::LemmyResult; +use serde::Deserialize; +use std::fmt::Debug; pub mod comment; pub mod community; @@ -43,9 +50,15 @@ pub(crate) fn read_from_string_or_source_opt( /// wrapped in Announce. If we simply receive this like any other federated object, overwrite the /// existing, local Post. In particular, it will set the field local = false, so that the object /// can't be fetched from the Activitypub HTTP endpoint anymore (which only serves local objects). -pub(crate) fn verify_is_remote_object(id: &Url, settings: &Settings) -> LemmyResult<()> { - let local_domain = settings.get_hostname_without_port()?; - if id.domain() == Some(&local_domain) { +pub(crate) fn verify_is_remote_object( + id: &ObjectId, + context: &Data, +) -> LemmyResult<()> +where + T: Object + Debug + Send + 'static, + for<'de2> ::Kind: Deserialize<'de2>, +{ + if id.is_local(context) { Err(anyhow!("cant accept local object from remote instance").into()) } else { Ok(()) diff --git a/crates/apub/src/objects/person.rs b/crates/apub/src/objects/person.rs index c99e09f72..1aac170d7 100644 --- a/crates/apub/src/objects/person.rs +++ b/crates/apub/src/objects/person.rs @@ -1,3 +1,4 @@ +use super::verify_is_remote_object; use crate::{ activities::GetActorType, check_apub_id_valid_with_strictness, @@ -137,6 +138,7 @@ impl Object for ApubPerson { check_slurs_opt(&person.name, slur_regex)?; verify_domains_match(person.id.inner(), expected_domain)?; + verify_is_remote_object(&person.id, context)?; check_apub_id_valid_with_strictness(person.id.inner(), false, context).await?; let bio = read_from_string_or_source_opt(&person.summary, &None, &person.source); diff --git a/crates/apub/src/objects/post.rs b/crates/apub/src/objects/post.rs index ba1e3f6a1..55af850e5 100644 --- a/crates/apub/src/objects/post.rs +++ b/crates/apub/src/objects/post.rs @@ -165,7 +165,7 @@ impl Object for ApubPost { // instance from the post author. if !page.is_mod_action(context).await? { verify_domains_match(page.id.inner(), expected_domain)?; - verify_is_remote_object(page.id.inner(), context.settings())?; + verify_is_remote_object(&page.id, context)?; }; let community = page.community(context).await?; diff --git a/crates/apub/src/objects/private_message.rs b/crates/apub/src/objects/private_message.rs index a78324633..4600c997a 100644 --- a/crates/apub/src/objects/private_message.rs +++ b/crates/apub/src/objects/private_message.rs @@ -1,3 +1,4 @@ +use super::verify_is_remote_object; use crate::{ check_apub_id_valid_with_strictness, objects::read_from_string_or_source, @@ -105,6 +106,7 @@ impl Object for ApubPrivateMessage { ) -> LemmyResult<()> { verify_domains_match(note.id.inner(), expected_domain)?; verify_domains_match(note.attributed_to.inner(), note.id.inner())?; + verify_is_remote_object(¬e.id, context)?; check_apub_id_valid_with_strictness(note.id.inner(), false, context).await?; let person = note.attributed_to.dereference(context).await?; diff --git a/crates/apub/src/protocol/objects/group.rs b/crates/apub/src/protocol/objects/group.rs index 30cfafb17..2ed884484 100644 --- a/crates/apub/src/protocol/objects/group.rs +++ b/crates/apub/src/protocol/objects/group.rs @@ -7,7 +7,7 @@ use crate::{ community_outbox::ApubCommunityOutbox, }, local_site_data_cached, - objects::{community::ApubCommunity, read_from_string_or_source_opt}, + objects::{community::ApubCommunity, read_from_string_or_source_opt, verify_is_remote_object}, protocol::{ objects::{Endpoints, LanguageTag}, ImageObject, @@ -15,6 +15,7 @@ use crate::{ }, }; use activitypub_federation::{ + config::Data, fetch::{collection_id::CollectionId, object_id::ObjectId}, kinds::actor::GroupType, protocol::{ @@ -75,10 +76,11 @@ impl Group { pub(crate) async fn verify( &self, expected_domain: &Url, - context: &LemmyContext, + context: &Data, ) -> LemmyResult<()> { check_apub_id_valid_with_strictness(self.id.inner(), true, context).await?; verify_domains_match(expected_domain, self.id.inner())?; + verify_is_remote_object(&self.id, context)?; let local_site_data = local_site_data_cached(&mut context.pool()).await?; let slur_regex = &local_site_opt_to_slur_regex(&local_site_data.local_site);