Skip to content

Commit

Permalink
Fixes LemmyNet#2900 - Checks slur regex to see if it is too permissiv…
Browse files Browse the repository at this point in the history
…e along with small validation organization
  • Loading branch information
ninanator committed Jun 16, 2023
1 parent becf75d commit a51ffe6
Show file tree
Hide file tree
Showing 4 changed files with 156 additions and 36 deletions.
11 changes: 2 additions & 9 deletions crates/api_common/src/utils.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
pub mod site_utils;

use crate::{
context::LemmyContext,
request::purge_image_from_pictrs,
Expand Down Expand Up @@ -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()) {
Expand Down
123 changes: 123 additions & 0 deletions crates/api_common/src/utils/site_utils.rs
Original file line number Diff line number Diff line change
@@ -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")));
}
}
37 changes: 18 additions & 19 deletions crates/api_crud/src/site/create.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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);
Expand All @@ -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)
Expand Down
21 changes: 13 additions & 8 deletions crates/api_crud/src/site/update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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)?)
Expand Down

0 comments on commit a51ffe6

Please sign in to comment.