Skip to content

Commit

Permalink
Forbid remote URLs for avatars/banners (fixes #1618)
Browse files Browse the repository at this point in the history
  • Loading branch information
Nutomic committed Mar 14, 2022
1 parent 550a93a commit 6fa9142
Show file tree
Hide file tree
Showing 8 changed files with 52 additions and 4 deletions.
4 changes: 4 additions & 0 deletions crates/api/src/local_user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -176,6 +177,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);
Expand Down
13 changes: 13 additions & 0 deletions crates/api_common/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ use lemmy_utils::{
LemmyError,
Sensitive,
};
use url::Url;

pub async fn blocking<F, T>(pool: &DbPool, f: F) -> Result<T, LemmyError>
where
Expand Down Expand Up @@ -629,3 +630,15 @@ pub async fn remove_user_data_in_community(

Ok(())
}

pub fn check_image_has_local_domain(url: &Option<String>) -> 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(())
}
3 changes: 3 additions & 0 deletions crates/api_crud/src/community/update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand Down
3 changes: 3 additions & 0 deletions crates/api_crud/src/site/update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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)?;
Expand Down
4 changes: 3 additions & 1 deletion crates/apub/src/objects/instance.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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)?;
Expand Down
13 changes: 12 additions & 1 deletion crates/apub/src/objects/mod.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -19,6 +22,14 @@ pub(crate) fn get_summary_from_string_or_source(
}
}

pub fn verify_image_domain_matches(a: &Url, b: &Option<ImageObject>) -> Result<(), LemmyError> {
if let Some(b) = b {
verify_domains_match(a, &b.url)
} else {
Ok(())
}
}

#[cfg(test)]
pub(crate) mod tests {
use actix::Actor;
Expand Down
8 changes: 7 additions & 1 deletion crates/apub/src/objects/person.rs
Original file line number Diff line number Diff line change
@@ -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},
Expand Down Expand Up @@ -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)?;
Expand Down
8 changes: 7 additions & 1 deletion crates/apub/src/protocol/objects/group.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)?;
Expand Down

0 comments on commit 6fa9142

Please sign in to comment.