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

@@ -2002,6 +2002,7 @@ dependencies = [
 name = "lemmy_utils"
 version = "0.1.0"
 dependencies = [
+ "actix-rt",
  "actix-web",
  "anyhow",
  "chrono",

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()),
+      icon,
-      banner: Some(data.banner.to_owned()),
+      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(),

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<String>) -> 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<Vec<String>, LemmyError> {
   let mut instances: Vec<String> = Vec::new();
 

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() {
+    check_optional_url(&data.url)?;
-      match Url::parse(url) {
-        Ok(_t) => (),
-        Err(_e) => return Err(APIError::err("invalid_url").into()),
-      }
-    }
 
     // Fetch Iframely and pictrs cached image
     let (iframely_title, iframely_description, iframely_html, pictrs_thumbnail) =

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());

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()?);
     }
 

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()?);
     }
 

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()?);
     }
 

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"] }