Fixes #2900 - Checks slur regex to see if it is too permissive

crates/api_common/src/utils.rs

@@ -1,3 +1,5 @@
+pub mod site_utils;
+
 use crate::{
   context::LemmyContext,
   request::purge_image_from_pictrs,
@@ -312,15 +314,6 @@ pub fn password_length_check(pass: &str) -> Result<(), LemmyError> {
   }
 }
 
-/// Checks the site description length
-pub fn site_description_length_check(description: &str) -> Result<(), LemmyError> {
-  if description.len() > 150 {
-    Err(LemmyError::from_message("site_description_length_overflow"))
-  } else {
-    Ok(())
-  }
-}
-
 /// Checks for a honeypot. If this field is filled, fail the rest of the function
 pub fn honeypot_check(honeypot: &Option<String>) -> Result<(), LemmyError> {
   if honeypot.is_some() && honeypot != &Some(String::new()) {

crates/api_common/src/utils/site_utils.rs

@@ -0,0 +1,123 @@
+/// Helper functions for manipulating parts of a site.
+use lemmy_utils::error::LemmyError;
+use regex::{Regex, RegexBuilder};
+
+const SITE_NAME_MAX_LENGTH: usize = 20;
+const SITE_DESCRIPTION_MAX_LENGTH: usize = 150;
+
+/// Attempts to build the regex and checks it for common errors before inserting into the DB.
+pub fn build_and_check_regex(regex_str: &Option<&str>) -> Result<Option<Regex>, LemmyError> {
+  regex_str.map_or_else(
+    || Ok(None::<Regex>),
+    |slurs| {
+      RegexBuilder::new(slurs)
+        .case_insensitive(true)
+        .build()
+        .map_err(|e| LemmyError::from_error_message(e, "invalid_regex"))
+        .and_then(|regex| {
+          // NOTE: It is difficult to know, in the universe of user-crafted regex, which ones
+          // may match against any string text. To keep it simple, we'll match the regex
+          // against an innocuous string - a single number - which should help catch a regex
+          // that accidentally matches against all strings.
+          if regex.is_match("1") {
+            return Err(LemmyError::from_message("permissive_slur"));
+          }
+
+          Ok(Some(regex))
+        })
+    },
+  )
+}
+
+/// Checks the site name length, the limit as defined in the DB.
+pub fn site_name_length_check(name: &str) -> Result<(), LemmyError> {
+  length_check(
+    name,
+    SITE_NAME_MAX_LENGTH,
+    String::from("site_name_length_overflow"),
+  )
+}
+
+/// Checks the site description length, the limit as defined in the DB.
+pub fn site_description_length_check(description: &str) -> Result<(), LemmyError> {
+  length_check(
+    description,
+    SITE_DESCRIPTION_MAX_LENGTH,
+    String::from("site_description_length_overflow"),
+  )
+}
+
+fn length_check(item: &str, max_length: usize, msg: String) -> Result<(), LemmyError> {
+  if item.len() > max_length {
+    Err(LemmyError::from_message(&msg))
+  } else {
+    Ok(())
+  }
+}
+
+#[cfg(test)]
+mod tests {
+  use crate::utils::site_utils::{
+    build_and_check_regex,
+    site_description_length_check,
+    site_name_length_check,
+  };
+
+  #[test]
+  fn test_valid_slur_regex() {
+    let valid_regexes = [&None, &Some("(foo|bar)")];
+
+    valid_regexes.iter().for_each(|regex| {
+      let result = build_and_check_regex(regex);
+
+      assert!(result.is_ok());
+    });
+  }
+
+  #[test]
+  fn test_too_permissive_slur_regex() {
+    let match_everything_regexes = [
+      &Some("["),          // Invalid regex
+      &Some("(foo|bar|)"), // Matches all values
+      &Some(".*"),         // Matches all values
+    ];
+
+    match_everything_regexes.iter().for_each(|regex| {
+      let result = build_and_check_regex(regex);
+
+      assert!(result.is_err(), "Testing regex: {:?}", regex);
+    });
+  }
+
+  #[test]
+  fn test_test_valid_site_name() {
+    let result = site_name_length_check("awesome.comm");
+
+    assert!(result.is_ok())
+  }
+
+  #[test]
+  fn test_test_invalid_site_name() {
+    let result = site_name_length_check("too long community name");
+
+    assert!(result.err().is_some_and(|error| error
+      .message
+      .is_some_and(|msg| msg == "site_name_length_overflow")));
+  }
+
+  #[test]
+  fn test_test_valid_site_description() {
+    let result = site_description_length_check("cool cats");
+
+    assert!(result.is_ok())
+  }
+
+  #[test]
+  fn test_test_invalid_site_description() {
+    let result = site_description_length_check(&(0..10001).map(|_| 'A').collect::<String>());
+
+    assert!(result.err().is_some_and(|error| error
+      .message
+      .is_some_and(|msg| msg == "site_description_length_overflow")));
+  }
+}

crates/api_crud/src/site/create.rs

@@ -8,9 +8,8 @@ use lemmy_api_common::{
     generate_site_inbox_url,
     is_admin,
     local_site_rate_limit_to_rate_limit_config,
-    local_site_to_slur_regex,
     local_user_view_from_jwt,
-    site_description_length_check,
+    site_utils::{build_and_check_regex, site_description_length_check, site_name_length_check},
   },
 };
 use lemmy_db_schema::{
@@ -41,31 +40,30 @@ impl PerformCrud for CreateSite {
   #[tracing::instrument(skip(context))]
   async fn perform(&self, context: &Data<LemmyContext>) -> Result<SiteResponse, LemmyError> {
     let data: &CreateSite = self;
-
+    let local_user_view = local_user_view_from_jwt(&data.auth, context).await?;
     let local_site = LocalSite::read(context.pool()).await?;
 
+    // BEGIN VALIDATION
+    // Make sure user is an admin; other types of users should not update site data...
+    is_admin(&local_user_view)?;
+
+    // Make sure the site hasn't already been set up...
     if local_site.site_setup {
       return Err(LemmyError::from_message("site_already_exists"));
     };
 
-    let local_user_view = local_user_view_from_jwt(&data.auth, context).await?;
-
-    let sidebar = diesel_option_overwrite(&data.sidebar);
-    let description = diesel_option_overwrite(&data.description);
-    let icon = diesel_option_overwrite_to_url(&data.icon)?;
-    let banner = diesel_option_overwrite_to_url(&data.banner)?;
+    // Check that the slur regex compiles, and returns the regex if valid...
+    let slur_regex = build_and_check_regex(&local_site.slur_filter_regex.as_deref())?;
 
-    let slur_regex = local_site_to_slur_regex(&local_site);
+    site_name_length_check(&data.name)?;
     check_slurs(&data.name, &slur_regex)?;
-    check_slurs_opt(&data.description, &slur_regex)?;
-
-    // Make sure user is an admin
-    is_admin(&local_user_view)?;
 
-    if let Some(Some(desc)) = &description {
+    if let Some(desc) = &data.description {
       site_description_length_check(desc)?;
+      check_slurs_opt(&data.description, &slur_regex)?;
     }
 
+    // Ensure that the sidebar has fewer than the max num characters...
     is_valid_body_field(&data.sidebar)?;
 
     let application_question = diesel_option_overwrite(&data.application_question);
@@ -75,16 +73,17 @@ impl PerformCrud for CreateSite {
         .registration_mode
         .unwrap_or(local_site.registration_mode),
     )?;
+    // END VALIDATION
 
     let actor_id: DbUrl = Url::parse(&context.settings().get_protocol_and_hostname())?.into();
     let inbox_url = Some(generate_site_inbox_url(&actor_id)?);
     let keypair = generate_actor_keypair()?;
     let site_form = SiteUpdateForm::builder()
       .name(Some(data.name.clone()))
-      .sidebar(sidebar)
-      .description(description)
-      .icon(icon)
-      .banner(banner)
+      .sidebar(diesel_option_overwrite(&data.sidebar))
+      .description(diesel_option_overwrite(&data.description))
+      .icon(diesel_option_overwrite_to_url(&data.icon)?)
+      .banner(diesel_option_overwrite_to_url(&data.banner)?)
       .actor_id(Some(actor_id))
       .last_refreshed_at(Some(naive_now()))
       .inbox_url(inbox_url)

crates/api_crud/src/site/update.rs

@@ -6,9 +6,8 @@ use lemmy_api_common::{
   utils::{
     is_admin,
     local_site_rate_limit_to_rate_limit_config,
-    local_site_to_slur_regex,
     local_user_view_from_jwt,
-    site_description_length_check,
+    site_utils::{build_and_check_regex, site_description_length_check, site_name_length_check},
   },
 };
 use lemmy_db_schema::{
@@ -45,18 +44,24 @@ impl PerformCrud for EditSite {
     let local_site = site_view.local_site;
     let site = site_view.site;
 
-    // Make sure user is an admin
+    // BEGIN VALIDATION
+    // Make sure user is an admin; other types of users should not update site data...
     is_admin(&local_user_view)?;
 
-    let slur_regex = local_site_to_slur_regex(&local_site);
+    // Check that the slur regex compiles, and return the regex if valid...
+    let slur_regex = build_and_check_regex(&local_site.slur_filter_regex.as_deref())?;
 
-    check_slurs_opt(&data.name, &slur_regex)?;
-    check_slurs_opt(&data.description, &slur_regex)?;
+    if let Some(name) = &data.name {
+      site_name_length_check(name)?;
+      check_slurs_opt(&data.name, &slur_regex)?;
+    }
 
     if let Some(desc) = &data.description {
       site_description_length_check(desc)?;
+      check_slurs_opt(&data.description, &slur_regex)?;
     }
 
+    // Ensure that the sidebar has fewer than the max num characters...
     is_valid_body_field(&data.sidebar)?;
 
     let application_question = diesel_option_overwrite(&data.application_question);
@@ -88,14 +93,14 @@ impl PerformCrud for EditSite {
         "cant_enable_private_instance_and_federation_together",
       ));
     }
+    // END VALIDATION
 
     if let Some(discussion_languages) = data.discussion_languages.clone() {
       SiteLanguage::update(context.pool(), discussion_languages.clone(), &site).await?;
     }
 
-    let name = data.name.clone();
     let site_form = SiteUpdateForm::builder()
-      .name(name)
+      .name(data.name.clone())
       .sidebar(diesel_option_overwrite(&data.sidebar))
       .description(diesel_option_overwrite(&data.description))
       .icon(diesel_option_overwrite_to_url(&data.icon)?)