From bcf7ec61092d4f6bba64b707df7e7e5fd191e935 Mon Sep 17 00:00:00 2001 From: Nutomic Date: Fri, 18 Mar 2022 15:46:58 +0000 Subject: [PATCH] Forbid remote URLs for avatars/banners (fixes #1618) (#2132) --- api_tests/src/shared.ts | 4 ---- crates/api/src/local_user.rs | 4 ++++ crates/api_common/src/lib.rs | 13 +++++++++++++ crates/api_crud/src/community/create.rs | 3 +++ crates/api_crud/src/community/update.rs | 3 +++ crates/api_crud/src/site/create.rs | 3 +++ crates/api_crud/src/site/update.rs | 3 +++ crates/apub/src/objects/instance.rs | 4 +++- crates/apub/src/objects/mod.rs | 13 ++++++++++++- crates/apub/src/objects/person.rs | 8 +++++++- crates/apub/src/protocol/objects/group.rs | 8 +++++++- 11 files changed, 58 insertions(+), 8 deletions(-) diff --git a/api_tests/src/shared.ts b/api_tests/src/shared.ts index 8c4e08ff4..da606b1fa 100644 --- a/api_tests/src/shared.ts +++ b/api_tests/src/shared.ts @@ -427,14 +427,10 @@ export async function createCommunity( name_: string = randomString(5) ): Promise { let description = 'a sample description'; - let icon = 'https://image.flaticon.com/icons/png/512/35/35896.png'; - let banner = 'https://image.flaticon.com/icons/png/512/35/35896.png'; let form: CreateCommunity = { name: name_, title: name_, description, - icon, - banner, nsfw: false, auth: api.auth, }; diff --git a/crates/api/src/local_user.rs b/crates/api/src/local_user.rs index 01ab2ed98..0b2a4c3aa 100644 --- a/crates/api/src/local_user.rs +++ b/crates/api/src/local_user.rs @@ -5,6 +5,7 @@ use captcha::{gen, Difficulty}; use chrono::Duration; use lemmy_api_common::{ blocking, + check_image_has_local_domain, check_registration_application, get_local_user_view_from_jwt, is_admin, @@ -175,6 +176,9 @@ impl Perform for SaveUserSettings { let local_user_view = get_local_user_view_from_jwt(&data.auth, context.pool(), context.secret()).await?; + check_image_has_local_domain(&data.avatar)?; + check_image_has_local_domain(&data.banner)?; + let avatar = diesel_option_overwrite_to_url(&data.avatar)?; let banner = diesel_option_overwrite_to_url(&data.banner)?; let bio = diesel_option_overwrite(&data.bio); diff --git a/crates/api_common/src/lib.rs b/crates/api_common/src/lib.rs index 59bbae053..a95365e9e 100644 --- a/crates/api_common/src/lib.rs +++ b/crates/api_common/src/lib.rs @@ -40,6 +40,7 @@ use lemmy_utils::{ LemmyError, Sensitive, }; +use url::Url; pub async fn blocking(pool: &DbPool, f: F) -> Result where @@ -623,3 +624,15 @@ pub async fn remove_user_data_in_community( Ok(()) } + +pub fn check_image_has_local_domain(url: &Option) -> Result<(), LemmyError> { + if let Some(url) = url { + let settings = Settings::get(); + let url = Url::parse(url)?; + let domain = url.domain().expect("url has domain"); + if domain != settings.hostname { + return Err(LemmyError::from_message("image_not_local")); + } + } + Ok(()) +} diff --git a/crates/api_crud/src/community/create.rs b/crates/api_crud/src/community/create.rs index 71e404d33..c76fe430d 100644 --- a/crates/api_crud/src/community/create.rs +++ b/crates/api_crud/src/community/create.rs @@ -2,6 +2,7 @@ use crate::PerformCrud; use actix_web::web::Data; use lemmy_api_common::{ blocking, + check_image_has_local_domain, community::{CommunityResponse, CreateCommunity}, get_local_user_view_from_jwt, is_admin, @@ -63,6 +64,8 @@ impl PerformCrud for CreateCommunity { check_slurs(&data.name, &context.settings().slur_regex())?; check_slurs(&data.title, &context.settings().slur_regex())?; check_slurs_opt(&data.description, &context.settings().slur_regex())?; + check_image_has_local_domain(&data.icon)?; + check_image_has_local_domain(&data.banner)?; if !is_valid_actor_name(&data.name, context.settings().actor_name_max_length) { return Err(LemmyError::from_message("invalid_community_name")); diff --git a/crates/api_crud/src/community/update.rs b/crates/api_crud/src/community/update.rs index b7352a624..f2a0c94c2 100644 --- a/crates/api_crud/src/community/update.rs +++ b/crates/api_crud/src/community/update.rs @@ -2,6 +2,7 @@ use crate::PerformCrud; use actix_web::web::Data; use lemmy_api_common::{ blocking, + check_image_has_local_domain, community::{CommunityResponse, EditCommunity, HideCommunity}, get_local_user_view_from_jwt, is_admin, @@ -37,6 +38,8 @@ impl PerformCrud for EditCommunity { check_slurs_opt(&data.title, &context.settings().slur_regex())?; check_slurs_opt(&data.description, &context.settings().slur_regex())?; + check_image_has_local_domain(&data.icon)?; + check_image_has_local_domain(&data.banner)?; // Verify its a mod (only mods can edit it) let community_id = data.community_id; diff --git a/crates/api_crud/src/site/create.rs b/crates/api_crud/src/site/create.rs index ddb4f0a0c..a38f211d8 100644 --- a/crates/api_crud/src/site/create.rs +++ b/crates/api_crud/src/site/create.rs @@ -2,6 +2,7 @@ use crate::PerformCrud; use actix_web::web::Data; use lemmy_api_common::{ blocking, + check_image_has_local_domain, get_local_user_view_from_jwt, is_admin, site::*, @@ -49,6 +50,8 @@ impl PerformCrud for CreateSite { check_slurs(&data.name, &context.settings().slur_regex())?; check_slurs_opt(&data.description, &context.settings().slur_regex())?; + check_image_has_local_domain(&data.icon)?; + check_image_has_local_domain(&data.banner)?; // Make sure user is an admin is_admin(&local_user_view)?; diff --git a/crates/api_crud/src/site/update.rs b/crates/api_crud/src/site/update.rs index 4e987de9d..11a23f268 100644 --- a/crates/api_crud/src/site/update.rs +++ b/crates/api_crud/src/site/update.rs @@ -2,6 +2,7 @@ use crate::PerformCrud; use actix_web::web::Data; use lemmy_api_common::{ blocking, + check_image_has_local_domain, get_local_user_view_from_jwt, is_admin, site::{EditSite, SiteResponse}, @@ -38,6 +39,8 @@ impl PerformCrud for EditSite { check_slurs_opt(&data.name, &context.settings().slur_regex())?; check_slurs_opt(&data.description, &context.settings().slur_regex())?; + check_image_has_local_domain(&data.icon)?; + check_image_has_local_domain(&data.banner)?; // Make sure user is an admin is_admin(&local_user_view)?; diff --git a/crates/apub/src/objects/instance.rs b/crates/apub/src/objects/instance.rs index a07ecdc17..f12696c49 100644 --- a/crates/apub/src/objects/instance.rs +++ b/crates/apub/src/objects/instance.rs @@ -1,6 +1,6 @@ use crate::{ check_is_apub_id_valid, - objects::get_summary_from_string_or_source, + objects::{get_summary_from_string_or_source, verify_image_domain_matches}, protocol::{objects::instance::Instance, ImageObject, Source}, }; use activitystreams_kinds::actor::ServiceType; @@ -103,6 +103,8 @@ impl ApubObject for ApubSite { ) -> Result<(), LemmyError> { check_is_apub_id_valid(apub.id.inner(), true, &data.settings())?; verify_domains_match(expected_domain, apub.id.inner())?; + verify_image_domain_matches(expected_domain, &apub.icon)?; + verify_image_domain_matches(expected_domain, &apub.image)?; let slur_regex = &data.settings().slur_regex(); check_slurs(&apub.name, slur_regex)?; diff --git a/crates/apub/src/objects/mod.rs b/crates/apub/src/objects/mod.rs index cf66beccd..6acbf9ad4 100644 --- a/crates/apub/src/objects/mod.rs +++ b/crates/apub/src/objects/mod.rs @@ -1,5 +1,8 @@ -use crate::protocol::Source; +use crate::protocol::{ImageObject, Source}; use html2md::parse_html; +use lemmy_apub_lib::verify::verify_domains_match; +use lemmy_utils::LemmyError; +use url::Url; pub mod comment; pub mod community; @@ -19,6 +22,14 @@ pub(crate) fn get_summary_from_string_or_source( } } +pub fn verify_image_domain_matches(a: &Url, b: &Option) -> Result<(), LemmyError> { + if let Some(b) = b { + verify_domains_match(a, &b.url) + } else { + Ok(()) + } +} + #[cfg(test)] pub(crate) mod tests { use actix::Actor; diff --git a/crates/apub/src/objects/person.rs b/crates/apub/src/objects/person.rs index 8eb7fe8b7..14e79e944 100644 --- a/crates/apub/src/objects/person.rs +++ b/crates/apub/src/objects/person.rs @@ -1,7 +1,11 @@ use crate::{ check_is_apub_id_valid, generate_outbox_url, - objects::{get_summary_from_string_or_source, instance::fetch_instance_actor_for_object}, + objects::{ + get_summary_from_string_or_source, + instance::fetch_instance_actor_for_object, + verify_image_domain_matches, + }, protocol::{ objects::{ person::{Person, UserTypes}, @@ -123,6 +127,8 @@ impl ApubObject for ApubPerson { ) -> Result<(), LemmyError> { verify_domains_match(person.id.inner(), expected_domain)?; check_is_apub_id_valid(person.id.inner(), false, &context.settings())?; + verify_image_domain_matches(expected_domain, &person.icon)?; + verify_image_domain_matches(expected_domain, &person.image)?; let slur_regex = &context.settings().slur_regex(); check_slurs(&person.preferred_username, slur_regex)?; diff --git a/crates/apub/src/protocol/objects/group.rs b/crates/apub/src/protocol/objects/group.rs index 5b564ff88..7820624d9 100644 --- a/crates/apub/src/protocol/objects/group.rs +++ b/crates/apub/src/protocol/objects/group.rs @@ -4,7 +4,11 @@ use crate::{ community_moderators::ApubCommunityModerators, community_outbox::ApubCommunityOutbox, }, - objects::{community::ApubCommunity, get_summary_from_string_or_source}, + objects::{ + community::ApubCommunity, + get_summary_from_string_or_source, + verify_image_domain_matches, + }, protocol::{objects::Endpoints, ImageObject, Source}, }; use activitystreams_kinds::actor::GroupType; @@ -58,6 +62,8 @@ impl Group { ) -> Result<(), LemmyError> { check_is_apub_id_valid(self.id.inner(), true, &context.settings())?; verify_domains_match(expected_domain, self.id.inner())?; + verify_image_domain_matches(expected_domain, &self.icon)?; + verify_image_domain_matches(expected_domain, &self.image)?; let slur_regex = &context.settings().slur_regex(); check_slurs(&self.preferred_username, slur_regex)?;