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.

diff --git a/Cargo.lock b/Cargo.lock
index 0e0e7e43e5151ddb6de0876804b2edca0f74091f..b67a96c2a602f7a92b555a458714628594191bf6 100644 (file)
--- 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 1d3e63a9c780232d6e50daaccdc819ae3f97173b..8b1a43bc59de8f4acd553e568beeba742129d4e1 100644 (file)
--- 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 1b9222f8c59799eadd4c00ee24d276ecd040677c..21be1819f89f2380f7347095fb4e15e0e2e3c724 100644 (file)
--- 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<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();
 

diff --git a/lemmy_api/src/post.rs b/lemmy_api/src/post.rs
index b3fbe8c99a7af69471837cef8e539562909f8b2b..c013cbbd00bc5a673cbbe2dc79a2005364e088a1 100644 (file)
--- 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 72f87789af3d4528108bba402dd59bfa1548d39b..58d119a69ec6095ac71f9ab49130a3550f7048d1 100644 (file)
--- 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 cf221fa2f3372a4e651d9d6262d30d74ec5c0896..6a4b1ee5cdd6a02ddab96aa79cce1dece2ef5c58 100644 (file)
--- 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 0c1aaf297bef6265326af044fc262f7b47b9101f..157dcbe275edf816643d17e640bbb1934941c117 100644 (file)
--- 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 ef314775f2a77f780c69bb1d1ccea7ff48198e29..944f7c1dac7aeeda4211f7465aef3875dd989d1d 100644 (file)
--- 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 25744b17d43c5c46ce4f16c73f6c076f7bc86c3e..e710847042e0da825d198ce2c5f38f7b3a30cc6a 100644 (file)
--- 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"] }