From 571c71392e6710c07edc315e51d7524afcdfa1fa Mon Sep 17 00:00:00 2001 From: Dessalines Date: Thu, 15 Oct 2020 14:23:56 -0400 Subject: [PATCH] Adding API and APUB URL checks for banners and icons. Fixes #1199 (#1200) * Adding API and APUB URL checks for banners and icons. Fixes #1199 * Adding a check optional url. * Missed a few. --- Cargo.lock | 1 + lemmy_api/src/community.rs | 23 ++++++++++++++++++++--- lemmy_api/src/lib.rs | 9 +++++++++ lemmy_api/src/post.rs | 9 ++------- lemmy_api/src/user.rs | 5 +++++ lemmy_apub/src/objects/community.rs | 6 +++--- lemmy_apub/src/objects/post.rs | 4 ++-- lemmy_apub/src/objects/user.rs | 4 ++-- lemmy_utils/Cargo.toml | 1 + 9 files changed, 45 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0e0e7e43e..b67a96c2a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2002,6 +2002,7 @@ dependencies = [ name = "lemmy_utils" version = "0.1.0" dependencies = [ + "actix-rt", "actix-web", "anyhow", "chrono", diff --git a/lemmy_api/src/community.rs b/lemmy_api/src/community.rs index 1d3e63a9c..8b1a43bc5 100644 --- a/lemmy_api/src/community.rs +++ b/lemmy_api/src/community.rs @@ -1,4 +1,11 @@ -use crate::{get_user_from_jwt, get_user_from_jwt_opt, is_admin, is_mod_or_admin, Perform}; +use crate::{ + check_optional_url, + get_user_from_jwt, + get_user_from_jwt_opt, + is_admin, + is_mod_or_admin, + Perform, +}; use actix_web::web::Data; use anyhow::Context; use lemmy_apub::ActorType; @@ -129,6 +136,13 @@ impl Perform for CreateCommunity { return Err(APIError::err("community_already_exists").into()); } + // Check to make sure the icon and banners are urls + let icon = diesel_option_overwrite(&data.icon); + let banner = diesel_option_overwrite(&data.banner); + + check_optional_url(&data.icon)?; + check_optional_url(&data.banner)?; + // When you create a community, make sure the user becomes a moderator and a follower let keypair = generate_actor_keypair()?; @@ -136,8 +150,8 @@ impl Perform for CreateCommunity { name: data.name.to_owned(), title: data.title.to_owned(), description: data.description.to_owned(), - icon: Some(data.icon.to_owned()), - banner: Some(data.banner.to_owned()), + icon, + banner, category_id: data.category_id, creator_id: user.id, removed: None, @@ -226,6 +240,9 @@ impl Perform for EditCommunity { let icon = diesel_option_overwrite(&data.icon); let banner = diesel_option_overwrite(&data.banner); + check_optional_url(&data.icon)?; + check_optional_url(&data.banner)?; + let community_form = CommunityForm { name: read_community.name, title: data.title.to_owned(), diff --git a/lemmy_api/src/lib.rs b/lemmy_api/src/lib.rs index 1b9222f8c..21be1819f 100644 --- a/lemmy_api/src/lib.rs +++ b/lemmy_api/src/lib.rs @@ -100,6 +100,15 @@ pub(in crate) async fn check_community_ban( } } +pub(in crate) fn check_optional_url(item: &Option) -> Result<(), LemmyError> { + if let Some(item) = &item { + if Url::parse(item).is_err() { + return Err(APIError::err("invalid_url").into()); + } + } + Ok(()) +} + pub(in crate) async fn linked_instances(pool: &DbPool) -> Result, LemmyError> { let mut instances: Vec = Vec::new(); diff --git a/lemmy_api/src/post.rs b/lemmy_api/src/post.rs index b3fbe8c99..c013cbbd0 100644 --- a/lemmy_api/src/post.rs +++ b/lemmy_api/src/post.rs @@ -1,5 +1,6 @@ use crate::{ check_community_ban, + check_optional_url, get_user_from_jwt, get_user_from_jwt_opt, is_mod_or_admin, @@ -36,7 +37,6 @@ use lemmy_websocket::{ UserOperation, }; use std::str::FromStr; -use url::Url; #[async_trait::async_trait(?Send)] impl Perform for CreatePost { @@ -59,12 +59,7 @@ impl Perform for CreatePost { check_community_ban(user.id, data.community_id, context.pool()).await?; - if let Some(url) = data.url.as_ref() { - match Url::parse(url) { - Ok(_t) => (), - Err(_e) => return Err(APIError::err("invalid_url").into()), - } - } + check_optional_url(&data.url)?; // Fetch Iframely and pictrs cached image let (iframely_title, iframely_description, iframely_html, pictrs_thumbnail) = diff --git a/lemmy_api/src/user.rs b/lemmy_api/src/user.rs index 72f87789a..58d119a69 100644 --- a/lemmy_api/src/user.rs +++ b/lemmy_api/src/user.rs @@ -1,5 +1,6 @@ use crate::{ captcha_espeak_wav_base64, + check_optional_url, claims::Claims, get_user_from_jwt, get_user_from_jwt_opt, @@ -347,6 +348,10 @@ impl Perform for SaveUserSettings { let preferred_username = diesel_option_overwrite(&data.preferred_username); let matrix_user_id = diesel_option_overwrite(&data.matrix_user_id); + // Check to make sure the avatar and banners are urls + check_optional_url(&data.avatar)?; + check_optional_url(&data.banner)?; + if let Some(Some(bio)) = &bio { if bio.chars().count() > 300 { return Err(APIError::err("bio_length_overflow").into()); diff --git a/lemmy_apub/src/objects/community.rs b/lemmy_apub/src/objects/community.rs index cf221fa2f..6a4b1ee5c 100644 --- a/lemmy_apub/src/objects/community.rs +++ b/lemmy_apub/src/objects/community.rs @@ -64,15 +64,15 @@ impl ToApub for Community { group.set_content(d); } - if let Some(icon) = &self.icon { + if let Some(icon_url) = &self.icon { let mut image = Image::new(); - image.set_url(icon.to_owned()); + image.set_url(Url::parse(icon_url)?); group.set_icon(image.into_any_base()?); } if let Some(banner_url) = &self.banner { let mut image = Image::new(); - image.set_url(banner_url.to_owned()); + image.set_url(Url::parse(banner_url)?); group.set_image(image.into_any_base()?); } diff --git a/lemmy_apub/src/objects/post.rs b/lemmy_apub/src/objects/post.rs index 0c1aaf297..157dcbe27 100644 --- a/lemmy_apub/src/objects/post.rs +++ b/lemmy_apub/src/objects/post.rs @@ -64,12 +64,12 @@ impl ToApub for Post { // https://github.com/LemmyNet/lemmy/issues/602 let url = self.url.as_ref().filter(|u| !u.is_empty()); if let Some(u) = url { - page.set_url(u.to_owned()); + page.set_url(Url::parse(u)?); } if let Some(thumbnail_url) = &self.thumbnail_url { let mut image = Image::new(); - image.set_url(thumbnail_url.to_string()); + image.set_url(Url::parse(thumbnail_url)?); page.set_image(image.into_any_base()?); } diff --git a/lemmy_apub/src/objects/user.rs b/lemmy_apub/src/objects/user.rs index ef314775f..944f7c1da 100644 --- a/lemmy_apub/src/objects/user.rs +++ b/lemmy_apub/src/objects/user.rs @@ -39,13 +39,13 @@ impl ToApub for User_ { if let Some(avatar_url) = &self.avatar { let mut image = Image::new(); - image.set_url(avatar_url.to_owned()); + image.set_url(Url::parse(avatar_url)?); person.set_icon(image.into_any_base()?); } if let Some(banner_url) = &self.banner { let mut image = Image::new(); - image.set_url(banner_url.to_owned()); + image.set_url(Url::parse(banner_url)?); person.set_image(image.into_any_base()?); } diff --git a/lemmy_utils/Cargo.toml b/lemmy_utils/Cargo.toml index 25744b17d..e71084704 100644 --- a/lemmy_utils/Cargo.toml +++ b/lemmy_utils/Cargo.toml @@ -25,5 +25,6 @@ lazy_static = "1.3" openssl = "0.10" url = { version = "2.1", features = ["serde"] } actix-web = { version = "3.0", default-features = false, features = ["rustls"] } +actix-rt = { version = "1.1", default-features = false } anyhow = "1.0" reqwest = { version = "0.10", features = ["json"] }