Only allow http(s) scheme for urls (ref #3505) (#3508)

With this change only http(s) schemes are allowed for post.url
field. This is checked for incoming api and federation requests.
Existing posts in database which are sent to clients are not
checked. Neither does it check urls in markdown.

diff --git a/crates/api_crud/src/post/create.rs b/crates/api_crud/src/post/create.rs
index 1dcc90241157fb06564b13e3b79ebf218ae98881..13d896b413cfa562ec81b5919f45d8d2674ba307 100644 (file)
--- a/crates/api_crud/src/post/create.rs
+++ b/crates/api_crud/src/post/create.rs
@@ -31,7 +31,7 @@ use lemmy_utils::{
   error::LemmyError,
   utils::{
     slurs::{check_slurs, check_slurs_opt},
   error::LemmyError,
   utils::{
     slurs::{check_slurs, check_slurs_opt},

-    validation::{clean_url_params, is_valid_body_field, is_valid_post_title},
+    validation::{check_url_scheme, clean_url_params, is_valid_body_field, is_valid_post_title},

   },
 };
 use tracing::{warn, Instrument};
   },
 };
 use tracing::{warn, Instrument};


@@ -58,6 +58,7 @@ impl PerformCrud for CreatePost {
 
     is_valid_post_title(&data.name)?;
     is_valid_body_field(&data.body, true)?;
 
     is_valid_post_title(&data.name)?;
     is_valid_body_field(&data.body, true)?;

+    check_url_scheme(&data.url)?;

 
     check_community_ban(local_user_view.person.id, data.community_id, context.pool()).await?;
     check_community_deleted_or_removed(data.community_id, context.pool()).await?;
 
     check_community_ban(local_user_view.person.id, data.community_id, context.pool()).await?;
     check_community_deleted_or_removed(data.community_id, context.pool()).await?;

diff --git a/crates/api_crud/src/post/update.rs b/crates/api_crud/src/post/update.rs
index 632bc5ae75af78483582a15605a8668ee6bf2c54..a8c5715bb46b2ab88445c3d2107a5d095f2a4d6b 100644 (file)
--- a/crates/api_crud/src/post/update.rs
+++ b/crates/api_crud/src/post/update.rs
@@ -20,7 +20,7 @@ use lemmy_utils::{
   error::LemmyError,
   utils::{
     slurs::check_slurs_opt,
   error::LemmyError,
   utils::{
     slurs::check_slurs_opt,

-    validation::{clean_url_params, is_valid_body_field, is_valid_post_title},
+    validation::{check_url_scheme, clean_url_params, is_valid_body_field, is_valid_post_title},

   },
 };
 
   },
 };
 


@@ -50,6 +50,7 @@ impl PerformCrud for EditPost {
     }
 
     is_valid_body_field(&data.body, true)?;
     }
 
     is_valid_body_field(&data.body, true)?;

+    check_url_scheme(&data.url)?;

 
     let post_id = data.post_id;
     let orig_post = Post::read(context.pool(), post_id).await?;
 
     let post_id = data.post_id;
     let orig_post = Post::read(context.pool(), post_id).await?;

diff --git a/crates/apub/src/objects/post.rs b/crates/apub/src/objects/post.rs
index 8064c771b70c85b6507310e2d5edfe75f6d27733..d73642be63969373df4fc14f422c78515be9aff2 100644 (file)
--- a/crates/apub/src/objects/post.rs
+++ b/crates/apub/src/objects/post.rs
@@ -44,6 +44,7 @@ use lemmy_utils::{
     markdown::markdown_to_html,
     slurs::{check_slurs_opt, remove_slurs},
     time::convert_datetime,
     markdown::markdown_to_html,
     slurs::{check_slurs_opt, remove_slurs},
     time::convert_datetime,

+    validation::check_url_scheme,

   },
 };
 use std::ops::Deref;
   },
 };
 use std::ops::Deref;


@@ -191,6 +192,7 @@ impl Object for ApubPost {
       } else {
         None
       };
       } else {
         None
       };

+      check_url_scheme(&url)?;

 
       let local_site = LocalSite::read(context.pool()).await.ok();
       let allow_sensitive = local_site_opt_to_sensitive(&local_site);
 
       let local_site = LocalSite::read(context.pool()).await.ok();
       let allow_sensitive = local_site_opt_to_sensitive(&local_site);

diff --git a/crates/utils/src/utils/validation.rs b/crates/utils/src/utils/validation.rs
index e43ddbbb54a85f0b8789c31f6a63de14549f5b38..b5ae5b74f3d473efd9474f002d41a6c15ab1d3dd 100644 (file)
--- a/crates/utils/src/utils/validation.rs
+++ b/crates/utils/src/utils/validation.rs
@@ -302,12 +302,22 @@ pub fn check_site_visibility_valid(
   Ok(())
 }
 
   Ok(())
 }
 

+pub fn check_url_scheme(url: &Option<Url>) -> LemmyResult<()> {
+  if let Some(url) = url {
+    if url.scheme() != "http" && url.scheme() != "https" {
+      return Err(LemmyError::from_message("invalid_url_scheme"));
+    }
+  }
+  Ok(())
+}
+

 #[cfg(test)]
 mod tests {
   use super::build_totp_2fa;
   use crate::utils::validation::{
     build_and_check_regex,
     check_site_visibility_valid,
 #[cfg(test)]
 mod tests {
   use super::build_totp_2fa;
   use crate::utils::validation::{
     build_and_check_regex,
     check_site_visibility_valid,

+    check_url_scheme,

     clean_url_params,
     generate_totp_2fa_secret,
     is_valid_actor_name,
     clean_url_params,
     generate_totp_2fa_secret,
     is_valid_actor_name,


@@ -519,4 +529,13 @@ mod tests {
     assert!(check_site_visibility_valid(false, false, &Some(true), &None).is_ok());
     assert!(check_site_visibility_valid(false, false, &None, &Some(true)).is_ok());
   }
     assert!(check_site_visibility_valid(false, false, &Some(true), &None).is_ok());
     assert!(check_site_visibility_valid(false, false, &None, &Some(true)).is_ok());
   }

+
+  #[test]
+  fn test_check_url_scheme() {
+    assert!(check_url_scheme(&None).is_ok());
+    assert!(check_url_scheme(&Some(Url::parse("http://example.com").unwrap())).is_ok());
+    assert!(check_url_scheme(&Some(Url::parse("https://example.com").unwrap())).is_ok());
+    assert!(check_url_scheme(&Some(Url::parse("ftp://example.com").unwrap())).is_err());
+    assert!(check_url_scheme(&Some(Url::parse("javascript:void").unwrap())).is_err());
+  }

 }
 }