From 71d61138bcc134d03f369f5396c627b2f5d075dc Mon Sep 17 00:00:00 2001 From: Nutomic Date: Wed, 6 Sep 2023 16:56:26 +0200 Subject: [PATCH] Replace ammonia lib with manual html escaping (fixes #3774) (#3938) * Replace ammonia lib with manual html escaping (fixes #3774) * prettier * clippy * remove sanitize unit test * fix tests * fix schema --- Cargo.lock | 20 ------- api_tests/src/post.spec.ts | 13 ++++- crates/api/src/comment_report/create.rs | 4 +- crates/api/src/community/ban.rs | 4 +- crates/api/src/community/hide.rs | 4 +- crates/api/src/local_user/ban_person.rs | 4 +- crates/api/src/local_user/save_settings.rs | 8 +-- crates/api/src/post_report/create.rs | 4 +- .../api/src/private_message_report/create.rs | 4 +- crates/api/src/site/purge/comment.rs | 4 +- crates/api/src/site/purge/community.rs | 9 +++- crates/api/src/site/purge/person.rs | 4 +- crates/api/src/site/purge/post.rs | 4 +- crates/api_common/Cargo.toml | 2 - crates/api_common/src/utils.rs | 52 ++++++++++--------- crates/api_crud/src/comment/create.rs | 4 +- crates/api_crud/src/comment/update.rs | 4 +- crates/api_crud/src/community/create.rs | 10 ++-- crates/api_crud/src/community/update.rs | 6 +-- crates/api_crud/src/custom_emoji/create.rs | 8 +-- crates/api_crud/src/custom_emoji/update.rs | 6 +-- crates/api_crud/src/post/create.rs | 12 ++--- crates/api_crud/src/post/update.rs | 10 ++-- crates/api_crud/src/private_message/create.rs | 4 +- crates/api_crud/src/private_message/update.rs | 4 +- crates/api_crud/src/site/create.rs | 16 +++--- crates/api_crud/src/site/update.rs | 14 ++--- crates/api_crud/src/user/create.rs | 4 +- .../apub/src/activities/block/block_user.rs | 6 +-- .../src/activities/block/undo_block_user.rs | 6 +-- .../apub/src/activities/community/report.rs | 6 +-- crates/apub/src/activities/deletion/delete.rs | 4 +- crates/apub/src/objects/comment.rs | 4 +- crates/apub/src/objects/instance.rs | 6 +-- crates/apub/src/objects/person.rs | 15 ++++-- crates/apub/src/objects/post.rs | 10 ++-- crates/apub/src/objects/private_message.rs | 4 +- crates/apub/src/protocol/objects/group.rs | 8 +-- crates/db_schema/src/schema.rs | 14 +---- 39 files changed, 157 insertions(+), 168 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f7d99de7f..cf731ca1a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -386,19 +386,6 @@ dependencies = [ "alloc-no-stdlib", ] -[[package]] -name = "ammonia" -version = "3.3.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "64e6d1c7838db705c9b756557ee27c384ce695a1c51a6fe528784cb1c6840170" -dependencies = [ - "html5ever", - "maplit", - "once_cell", - "tendril", - "url", -] - [[package]] name = "android-tzdata" version = "0.1.1" @@ -2613,7 +2600,6 @@ version = "0.18.1" dependencies = [ "activitypub_federation", "actix-web", - "ammonia", "anyhow", "chrono", "encoding", @@ -3028,12 +3014,6 @@ dependencies = [ "libc", ] -[[package]] -name = "maplit" -version = "1.0.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3e2e65a1a2e43cfcb47a895c4c8b10d1f4a61097f9f254f183aee60cad9c651d" - [[package]] name = "markdown-it" version = "0.5.1" diff --git a/api_tests/src/post.spec.ts b/api_tests/src/post.spec.ts index 42173dba8..52fe72d3d 100644 --- a/api_tests/src/post.spec.ts +++ b/api_tests/src/post.spec.ts @@ -513,7 +513,7 @@ test("Sanitize HTML", async () => { } let name = randomString(5); - let body = " hello"; + let body = " hello &\"'"; let form: CreatePost = { name, body, @@ -521,5 +521,14 @@ test("Sanitize HTML", async () => { community_id: betaCommunity.community.id, }; let post = await beta.client.createPost(form); - expect(post.post_view.post.body).toBe(" hello"); + // first escaping for the api + expect(post.post_view.post.body).toBe( + "<script>alert('xss');</script> hello &"'", + ); + + let alphaPost = (await resolvePost(alpha, post.post_view.post)).post; + // second escaping over federation, avoid double escape of & + expect(alphaPost?.post.body).toBe( + "<script>alert('xss');</script> hello &"'", + ); }); diff --git a/crates/api/src/comment_report/create.rs b/crates/api/src/comment_report/create.rs index 2ea973d3c..e493d60a3 100644 --- a/crates/api/src/comment_report/create.rs +++ b/crates/api/src/comment_report/create.rs @@ -8,7 +8,7 @@ use lemmy_api_common::{ utils::{ check_community_ban, local_user_view_from_jwt, - sanitize_html, + sanitize_html_api, send_new_report_email_to_admins, }, }; @@ -31,7 +31,7 @@ pub async fn create_comment_report( let local_user_view = local_user_view_from_jwt(&data.auth, &context).await?; let local_site = LocalSite::read(&mut context.pool()).await?; - let reason = sanitize_html(data.reason.trim()); + let reason = sanitize_html_api(data.reason.trim()); check_report_reason(&reason, &local_site)?; let person_id = local_user_view.person.id; diff --git a/crates/api/src/community/ban.rs b/crates/api/src/community/ban.rs index d04a8a0a9..e969e9b78 100644 --- a/crates/api/src/community/ban.rs +++ b/crates/api/src/community/ban.rs @@ -8,7 +8,7 @@ use lemmy_api_common::{ is_mod_or_admin, local_user_view_from_jwt, remove_user_data_in_community, - sanitize_html_opt, + sanitize_html_api_opt, }, }; use lemmy_db_schema::{ @@ -86,7 +86,7 @@ pub async fn ban_from_community( mod_person_id: local_user_view.person.id, other_person_id: data.person_id, community_id: data.community_id, - reason: sanitize_html_opt(&data.reason), + reason: sanitize_html_api_opt(&data.reason), banned: Some(data.ban), expires, }; diff --git a/crates/api/src/community/hide.rs b/crates/api/src/community/hide.rs index 10ca6372a..8fd270181 100644 --- a/crates/api/src/community/hide.rs +++ b/crates/api/src/community/hide.rs @@ -5,7 +5,7 @@ use lemmy_api_common::{ community::{CommunityResponse, HideCommunity}, context::LemmyContext, send_activity::{ActivityChannel, SendActivityData}, - utils::{is_admin, local_user_view_from_jwt, sanitize_html_opt}, + utils::{is_admin, local_user_view_from_jwt, sanitize_html_api_opt}, }; use lemmy_db_schema::{ source::{ @@ -33,7 +33,7 @@ pub async fn hide_community( let mod_hide_community_form = ModHideCommunityForm { community_id: data.community_id, mod_person_id: local_user_view.person.id, - reason: sanitize_html_opt(&data.reason), + reason: sanitize_html_api_opt(&data.reason), hidden: Some(data.hidden), }; diff --git a/crates/api/src/local_user/ban_person.rs b/crates/api/src/local_user/ban_person.rs index 3a3a240da..3a83d6c6e 100644 --- a/crates/api/src/local_user/ban_person.rs +++ b/crates/api/src/local_user/ban_person.rs @@ -4,7 +4,7 @@ use lemmy_api_common::{ context::LemmyContext, person::{BanPerson, BanPersonResponse}, send_activity::{ActivityChannel, SendActivityData}, - utils::{is_admin, local_user_view_from_jwt, remove_user_data, sanitize_html_opt}, + utils::{is_admin, local_user_view_from_jwt, remove_user_data, sanitize_html_api_opt}, }; use lemmy_db_schema::{ source::{ @@ -54,7 +54,7 @@ pub async fn ban_from_site( let form = ModBanForm { mod_person_id: local_user_view.person.id, other_person_id: data.person_id, - reason: sanitize_html_opt(&data.reason), + reason: sanitize_html_api_opt(&data.reason), banned: Some(data.ban), expires, }; diff --git a/crates/api/src/local_user/save_settings.rs b/crates/api/src/local_user/save_settings.rs index c45c69f7f..8368eada0 100644 --- a/crates/api/src/local_user/save_settings.rs +++ b/crates/api/src/local_user/save_settings.rs @@ -2,7 +2,7 @@ use actix_web::web::{Data, Json}; use lemmy_api_common::{ context::LemmyContext, person::{LoginResponse, SaveUserSettings}, - utils::{local_user_view_from_jwt, sanitize_html_opt, send_verification_email}, + utils::{local_user_view_from_jwt, sanitize_html_api_opt, send_verification_email}, }; use lemmy_db_schema::{ source::{ @@ -34,8 +34,8 @@ pub async fn save_user_settings( let local_user_view = local_user_view_from_jwt(&data.auth, &context).await?; let site_view = SiteView::read_local(&mut context.pool()).await?; - let bio = sanitize_html_opt(&data.bio); - let display_name = sanitize_html_opt(&data.display_name); + let bio = sanitize_html_api_opt(&data.bio); + let display_name = sanitize_html_api_opt(&data.display_name); let avatar = diesel_option_overwrite_to_url(&data.avatar)?; let banner = diesel_option_overwrite_to_url(&data.banner)?; @@ -85,7 +85,7 @@ pub async fn save_user_settings( let person_id = local_user_view.person.id; let default_listing_type = data.default_listing_type; let default_sort_type = data.default_sort_type; - let theme = sanitize_html_opt(&data.theme); + let theme = sanitize_html_api_opt(&data.theme); let person_form = PersonUpdateForm { display_name, diff --git a/crates/api/src/post_report/create.rs b/crates/api/src/post_report/create.rs index d4100a84e..8a5162a60 100644 --- a/crates/api/src/post_report/create.rs +++ b/crates/api/src/post_report/create.rs @@ -8,7 +8,7 @@ use lemmy_api_common::{ utils::{ check_community_ban, local_user_view_from_jwt, - sanitize_html, + sanitize_html_api, send_new_report_email_to_admins, }, }; @@ -31,7 +31,7 @@ pub async fn create_post_report( let local_user_view = local_user_view_from_jwt(&data.auth, &context).await?; let local_site = LocalSite::read(&mut context.pool()).await?; - let reason = sanitize_html(data.reason.trim()); + let reason = sanitize_html_api(data.reason.trim()); check_report_reason(&reason, &local_site)?; let person_id = local_user_view.person.id; diff --git a/crates/api/src/private_message_report/create.rs b/crates/api/src/private_message_report/create.rs index e1479d3c3..ed19fdc35 100644 --- a/crates/api/src/private_message_report/create.rs +++ b/crates/api/src/private_message_report/create.rs @@ -3,7 +3,7 @@ use actix_web::web::{Data, Json}; use lemmy_api_common::{ context::LemmyContext, private_message::{CreatePrivateMessageReport, PrivateMessageReportResponse}, - utils::{local_user_view_from_jwt, sanitize_html, send_new_report_email_to_admins}, + utils::{local_user_view_from_jwt, sanitize_html_api, send_new_report_email_to_admins}, }; use lemmy_db_schema::{ source::{ @@ -24,7 +24,7 @@ pub async fn create_pm_report( let local_user_view = local_user_view_from_jwt(&data.auth, &context).await?; let local_site = LocalSite::read(&mut context.pool()).await?; - let reason = sanitize_html(data.reason.trim()); + let reason = sanitize_html_api(data.reason.trim()); check_report_reason(&reason, &local_site)?; let person_id = local_user_view.person.id; diff --git a/crates/api/src/site/purge/comment.rs b/crates/api/src/site/purge/comment.rs index 391be98d7..fa0973cf1 100644 --- a/crates/api/src/site/purge/comment.rs +++ b/crates/api/src/site/purge/comment.rs @@ -2,7 +2,7 @@ use actix_web::web::{Data, Json}; use lemmy_api_common::{ context::LemmyContext, site::{PurgeComment, PurgeItemResponse}, - utils::{is_admin, local_user_view_from_jwt, sanitize_html_opt}, + utils::{is_admin, local_user_view_from_jwt, sanitize_html_api_opt}, }; use lemmy_db_schema::{ source::{ @@ -35,7 +35,7 @@ pub async fn purge_comment( Comment::delete(&mut context.pool(), comment_id).await?; // Mod tables - let reason = sanitize_html_opt(&data.reason); + let reason = sanitize_html_api_opt(&data.reason); let form = AdminPurgeCommentForm { admin_person_id: local_user_view.person.id, reason, diff --git a/crates/api/src/site/purge/community.rs b/crates/api/src/site/purge/community.rs index 60c1142c3..8e609501a 100644 --- a/crates/api/src/site/purge/community.rs +++ b/crates/api/src/site/purge/community.rs @@ -3,7 +3,12 @@ use lemmy_api_common::{ context::LemmyContext, request::purge_image_from_pictrs, site::{PurgeCommunity, PurgeItemResponse}, - utils::{is_admin, local_user_view_from_jwt, purge_image_posts_for_community, sanitize_html_opt}, + utils::{ + is_admin, + local_user_view_from_jwt, + purge_image_posts_for_community, + sanitize_html_api_opt, + }, }; use lemmy_db_schema::{ source::{ @@ -42,7 +47,7 @@ pub async fn purge_community( Community::delete(&mut context.pool(), community_id).await?; // Mod tables - let reason = sanitize_html_opt(&data.reason); + let reason = sanitize_html_api_opt(&data.reason); let form = AdminPurgeCommunityForm { admin_person_id: local_user_view.person.id, reason, diff --git a/crates/api/src/site/purge/person.rs b/crates/api/src/site/purge/person.rs index ed38e2354..b2968ec31 100644 --- a/crates/api/src/site/purge/person.rs +++ b/crates/api/src/site/purge/person.rs @@ -3,7 +3,7 @@ use lemmy_api_common::{ context::LemmyContext, request::delete_image_from_pictrs, site::{PurgeItemResponse, PurgePerson}, - utils::{is_admin, local_user_view_from_jwt, sanitize_html_opt}, + utils::{is_admin, local_user_view_from_jwt, sanitize_html_api_opt}, }; use lemmy_db_schema::{ source::{ @@ -42,7 +42,7 @@ pub async fn purge_person( Person::delete(&mut context.pool(), person_id).await?; // Mod tables - let reason = sanitize_html_opt(&data.reason); + let reason = sanitize_html_api_opt(&data.reason); let form = AdminPurgePersonForm { admin_person_id: local_user_view.person.id, reason, diff --git a/crates/api/src/site/purge/post.rs b/crates/api/src/site/purge/post.rs index a6e6c1825..53dbf44e3 100644 --- a/crates/api/src/site/purge/post.rs +++ b/crates/api/src/site/purge/post.rs @@ -3,7 +3,7 @@ use lemmy_api_common::{ context::LemmyContext, request::purge_image_from_pictrs, site::{PurgeItemResponse, PurgePost}, - utils::{is_admin, local_user_view_from_jwt, sanitize_html_opt}, + utils::{is_admin, local_user_view_from_jwt, sanitize_html_api_opt}, }; use lemmy_db_schema::{ source::{ @@ -43,7 +43,7 @@ pub async fn purge_post( Post::delete(&mut context.pool(), post_id).await?; // Mod tables - let reason = sanitize_html_opt(&data.reason); + let reason = sanitize_html_api_opt(&data.reason); let form = AdminPurgePostForm { admin_person_id: local_user_view.person.id, reason, diff --git a/crates/api_common/Cargo.toml b/crates/api_common/Cargo.toml index d74acd136..8a23a4cb2 100644 --- a/crates/api_common/Cargo.toml +++ b/crates/api_common/Cargo.toml @@ -34,7 +34,6 @@ full = [ "actix-web", "futures", "once_cell", - "ammonia", ] [dependencies] @@ -67,4 +66,3 @@ once_cell = { workspace = true, optional = true } actix-web = { workspace = true, optional = true } # necessary for wasmt compilation getrandom = { version = "0.2.10", features = ["js"] } -ammonia = { version = "3.3.0", optional = true } diff --git a/crates/api_common/src/utils.rs b/crates/api_common/src/utils.rs index 3a1f1f945..9d8181a2e 100644 --- a/crates/api_common/src/utils.rs +++ b/crates/api_common/src/utils.rs @@ -798,21 +798,35 @@ pub fn generate_moderators_url(community_id: &DbUrl) -> Result String { - ammonia::Builder::default() - .rm_tags(&["a", "img"]) - .clean(data) - .to_string() - // restore markdown quotes - .replace(">", ">") - // restore white space - .replace(" ", " ") +/// Replace special HTML characters in API parameters to prevent XSS attacks. +/// +/// Taken from https://github.com/OWASP/CheatSheetSeries/blob/master/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.md#output-encoding-for-html-contexts +/// +/// `>` is left in place because it is interpreted as markdown quote. +pub fn sanitize_html_api(data: &str) -> String { + data + .replace('&', "&") + .replace('<', "<") + .replace('\"', """) + .replace('\'', "'") } -pub fn sanitize_html_opt(data: &Option) -> Option { - data.as_ref().map(|d| sanitize_html(d)) +pub fn sanitize_html_api_opt(data: &Option) -> Option { + data.as_ref().map(|d| sanitize_html_api(d)) +} + +/// Replace special HTML characters in federation parameters to prevent XSS attacks. +/// +/// Unlike [sanitize_html_api()] it leaves `&` in place to avoid double escaping. +pub fn sanitize_html_federation(data: &str) -> String { + data + .replace('<', "<") + .replace('\"', """) + .replace('\'', "'") +} + +pub fn sanitize_html_federation_opt(data: &Option) -> Option { + data.as_ref().map(|d| sanitize_html_federation(d)) } #[cfg(test)] @@ -820,7 +834,7 @@ mod tests { #![allow(clippy::unwrap_used)] #![allow(clippy::indexing_slicing)] - use crate::utils::{honeypot_check, password_length_check, sanitize_html}; + use crate::utils::{honeypot_check, password_length_check}; #[test] #[rustfmt::skip] @@ -838,14 +852,4 @@ mod tests { assert!(honeypot_check(&Some("1".to_string())).is_err()); assert!(honeypot_check(&Some("message".to_string())).is_err()); } - - #[test] - fn test_sanitize_html() { - let sanitized = sanitize_html(" hello"); - assert_eq!(sanitized, " hello"); - let sanitized = sanitize_html(" test"); - assert_eq!(sanitized, " test"); - let sanitized = sanitize_html("Hello World"); - assert_eq!(sanitized, "Hello World"); - } } diff --git a/crates/api_crud/src/comment/create.rs b/crates/api_crud/src/comment/create.rs index 00762a31b..3ba19d1b9 100644 --- a/crates/api_crud/src/comment/create.rs +++ b/crates/api_crud/src/comment/create.rs @@ -13,7 +13,7 @@ use lemmy_api_common::{ get_post, local_site_to_slur_regex, local_user_view_from_jwt, - sanitize_html, + sanitize_html_api, EndpointType, }, }; @@ -52,7 +52,7 @@ pub async fn create_comment( &local_site_to_slur_regex(&local_site), ); is_valid_body_field(&Some(content.clone()), false)?; - let content = sanitize_html(&content); + let content = sanitize_html_api(&content); // Check for a community ban let post_id = data.post_id; diff --git a/crates/api_crud/src/comment/update.rs b/crates/api_crud/src/comment/update.rs index 16f56092f..bf6bdff84 100644 --- a/crates/api_crud/src/comment/update.rs +++ b/crates/api_crud/src/comment/update.rs @@ -9,7 +9,7 @@ use lemmy_api_common::{ check_community_ban, local_site_to_slur_regex, local_user_view_from_jwt, - sanitize_html_opt, + sanitize_html_api_opt, }, }; use lemmy_db_schema::{ @@ -68,7 +68,7 @@ pub async fn update_comment( .as_ref() .map(|c| remove_slurs(c, &local_site_to_slur_regex(&local_site))); is_valid_body_field(&content, false)?; - let content = sanitize_html_opt(&content); + let content = sanitize_html_api_opt(&content); let comment_id = data.comment_id; let form = CommentUpdateForm { diff --git a/crates/api_crud/src/community/create.rs b/crates/api_crud/src/community/create.rs index ef6a317e9..d6c07c128 100644 --- a/crates/api_crud/src/community/create.rs +++ b/crates/api_crud/src/community/create.rs @@ -12,8 +12,8 @@ use lemmy_api_common::{ is_admin, local_site_to_slur_regex, local_user_view_from_jwt, - sanitize_html, - sanitize_html_opt, + sanitize_html_api, + sanitize_html_api_opt, EndpointType, }, }; @@ -58,9 +58,9 @@ pub async fn create_community( let icon = diesel_option_overwrite_to_url_create(&data.icon)?; let banner = diesel_option_overwrite_to_url_create(&data.banner)?; - let name = sanitize_html(&data.name); - let title = sanitize_html(&data.title); - let description = sanitize_html_opt(&data.description); + let name = sanitize_html_api(&data.name); + let title = sanitize_html_api(&data.title); + let description = sanitize_html_api_opt(&data.description); let slur_regex = local_site_to_slur_regex(&local_site); check_slurs(&name, &slur_regex)?; diff --git a/crates/api_crud/src/community/update.rs b/crates/api_crud/src/community/update.rs index b55693598..122cab94e 100644 --- a/crates/api_crud/src/community/update.rs +++ b/crates/api_crud/src/community/update.rs @@ -5,7 +5,7 @@ use lemmy_api_common::{ community::{CommunityResponse, EditCommunity}, context::LemmyContext, send_activity::{ActivityChannel, SendActivityData}, - utils::{local_site_to_slur_regex, local_user_view_from_jwt, sanitize_html_opt}, + utils::{local_site_to_slur_regex, local_user_view_from_jwt, sanitize_html_api_opt}, }; use lemmy_db_schema::{ newtypes::PersonId, @@ -36,8 +36,8 @@ pub async fn update_community( check_slurs_opt(&data.description, &slur_regex)?; is_valid_body_field(&data.description, false)?; - let title = sanitize_html_opt(&data.title); - let description = sanitize_html_opt(&data.description); + let title = sanitize_html_api_opt(&data.title); + let description = sanitize_html_api_opt(&data.description); let icon = diesel_option_overwrite_to_url(&data.icon)?; let banner = diesel_option_overwrite_to_url(&data.banner)?; diff --git a/crates/api_crud/src/custom_emoji/create.rs b/crates/api_crud/src/custom_emoji/create.rs index 589174455..046003627 100644 --- a/crates/api_crud/src/custom_emoji/create.rs +++ b/crates/api_crud/src/custom_emoji/create.rs @@ -3,7 +3,7 @@ use actix_web::web::Json; use lemmy_api_common::{ context::LemmyContext, custom_emoji::{CreateCustomEmoji, CustomEmojiResponse}, - utils::{is_admin, local_user_view_from_jwt, sanitize_html}, + utils::{is_admin, local_user_view_from_jwt, sanitize_html_api}, }; use lemmy_db_schema::source::{ custom_emoji::{CustomEmoji, CustomEmojiInsertForm}, @@ -24,9 +24,9 @@ pub async fn create_custom_emoji( // Make sure user is an admin is_admin(&local_user_view)?; - let shortcode = sanitize_html(data.shortcode.to_lowercase().trim()); - let alt_text = sanitize_html(&data.alt_text); - let category = sanitize_html(&data.category); + let shortcode = sanitize_html_api(data.shortcode.to_lowercase().trim()); + let alt_text = sanitize_html_api(&data.alt_text); + let category = sanitize_html_api(&data.category); let emoji_form = CustomEmojiInsertForm::builder() .local_site_id(local_site.id) diff --git a/crates/api_crud/src/custom_emoji/update.rs b/crates/api_crud/src/custom_emoji/update.rs index 7a2056895..965250feb 100644 --- a/crates/api_crud/src/custom_emoji/update.rs +++ b/crates/api_crud/src/custom_emoji/update.rs @@ -3,7 +3,7 @@ use actix_web::web::Json; use lemmy_api_common::{ context::LemmyContext, custom_emoji::{CustomEmojiResponse, EditCustomEmoji}, - utils::{is_admin, local_user_view_from_jwt, sanitize_html}, + utils::{is_admin, local_user_view_from_jwt, sanitize_html_api}, }; use lemmy_db_schema::source::{ custom_emoji::{CustomEmoji, CustomEmojiUpdateForm}, @@ -24,8 +24,8 @@ pub async fn update_custom_emoji( // Make sure user is an admin is_admin(&local_user_view)?; - let alt_text = sanitize_html(&data.alt_text); - let category = sanitize_html(&data.category); + let alt_text = sanitize_html_api(&data.alt_text); + let category = sanitize_html_api(&data.category); let emoji_form = CustomEmojiUpdateForm::builder() .local_site_id(local_site.id) diff --git a/crates/api_crud/src/post/create.rs b/crates/api_crud/src/post/create.rs index 9d27f5d62..d0b0f368c 100644 --- a/crates/api_crud/src/post/create.rs +++ b/crates/api_crud/src/post/create.rs @@ -14,8 +14,8 @@ use lemmy_api_common::{ local_site_to_slur_regex, local_user_view_from_jwt, mark_post_as_read, - sanitize_html, - sanitize_html_opt, + sanitize_html_api, + sanitize_html_api_opt, EndpointType, }, }; @@ -93,10 +93,10 @@ pub async fn create_post( .map(|u| (u.title, u.description, u.embed_video_url)) .unwrap_or_default(); - let name = sanitize_html(data.name.trim()); - let body = sanitize_html_opt(&data.body); - let embed_title = sanitize_html_opt(&embed_title); - let embed_description = sanitize_html_opt(&embed_description); + let name = sanitize_html_api(data.name.trim()); + let body = sanitize_html_api_opt(&data.body); + let embed_title = sanitize_html_api_opt(&embed_title); + let embed_description = sanitize_html_api_opt(&embed_description); // Only need to check if language is allowed in case user set it explicitly. When using default // language, it already only returns allowed languages. diff --git a/crates/api_crud/src/post/update.rs b/crates/api_crud/src/post/update.rs index c475d7a26..880991b8e 100644 --- a/crates/api_crud/src/post/update.rs +++ b/crates/api_crud/src/post/update.rs @@ -10,7 +10,7 @@ use lemmy_api_common::{ check_community_ban, local_site_to_slur_regex, local_user_view_from_jwt, - sanitize_html_opt, + sanitize_html_api_opt, }, }; use lemmy_db_schema::{ @@ -79,11 +79,11 @@ pub async fn update_post( .map(|u| (Some(u.title), Some(u.description), Some(u.embed_video_url))) .unwrap_or_default(); - let name = sanitize_html_opt(&data.name); - let body = sanitize_html_opt(&data.body); + let name = sanitize_html_api_opt(&data.name); + let body = sanitize_html_api_opt(&data.body); let body = diesel_option_overwrite(body); - let embed_title = embed_title.map(|e| sanitize_html_opt(&e)); - let embed_description = embed_description.map(|e| sanitize_html_opt(&e)); + let embed_title = embed_title.map(|e| sanitize_html_api_opt(&e)); + let embed_description = embed_description.map(|e| sanitize_html_api_opt(&e)); let language_id = data.language_id; CommunityLanguage::is_allowed_community_language( diff --git a/crates/api_crud/src/private_message/create.rs b/crates/api_crud/src/private_message/create.rs index f5e4b15ab..1e22935ce 100644 --- a/crates/api_crud/src/private_message/create.rs +++ b/crates/api_crud/src/private_message/create.rs @@ -10,7 +10,7 @@ use lemmy_api_common::{ get_interface_language, local_site_to_slur_regex, local_user_view_from_jwt, - sanitize_html, + sanitize_html_api, send_email_to_user, EndpointType, }, @@ -36,7 +36,7 @@ pub async fn create_private_message( let local_user_view = local_user_view_from_jwt(&data.auth, &context).await?; let local_site = LocalSite::read(&mut context.pool()).await?; - let content = sanitize_html(&data.content); + let content = sanitize_html_api(&data.content); let content = remove_slurs(&content, &local_site_to_slur_regex(&local_site)); is_valid_body_field(&Some(content.clone()), false)?; diff --git a/crates/api_crud/src/private_message/update.rs b/crates/api_crud/src/private_message/update.rs index 44cc1ab3b..258dd8bcb 100644 --- a/crates/api_crud/src/private_message/update.rs +++ b/crates/api_crud/src/private_message/update.rs @@ -4,7 +4,7 @@ use lemmy_api_common::{ context::LemmyContext, private_message::{EditPrivateMessage, PrivateMessageResponse}, send_activity::{ActivityChannel, SendActivityData}, - utils::{local_site_to_slur_regex, local_user_view_from_jwt, sanitize_html}, + utils::{local_site_to_slur_regex, local_user_view_from_jwt, sanitize_html_api}, }; use lemmy_db_schema::{ source::{ @@ -36,7 +36,7 @@ pub async fn update_private_message( } // Doing the update - let content = sanitize_html(&data.content); + let content = sanitize_html_api(&data.content); let content = remove_slurs(&content, &local_site_to_slur_regex(&local_site)); is_valid_body_field(&Some(content.clone()), false)?; diff --git a/crates/api_crud/src/site/create.rs b/crates/api_crud/src/site/create.rs index 78bbb64ef..5cf9a04a7 100644 --- a/crates/api_crud/src/site/create.rs +++ b/crates/api_crud/src/site/create.rs @@ -9,8 +9,8 @@ use lemmy_api_common::{ is_admin, local_site_rate_limit_to_rate_limit_config, local_user_view_from_jwt, - sanitize_html, - sanitize_html_opt, + sanitize_html_api, + sanitize_html_api_opt, }, }; use lemmy_db_schema::{ @@ -56,9 +56,9 @@ pub async fn create_site( 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 name = sanitize_html(&data.name); - let sidebar = sanitize_html_opt(&data.sidebar); - let description = sanitize_html_opt(&data.description); + let name = sanitize_html_api(&data.name); + let sidebar = sanitize_html_api_opt(&data.sidebar); + let description = sanitize_html_api_opt(&data.description); let site_form = SiteUpdateForm { name: Some(name), @@ -78,9 +78,9 @@ pub async fn create_site( Site::update(&mut context.pool(), site_id, &site_form).await?; - let application_question = sanitize_html_opt(&data.application_question); - let default_theme = sanitize_html_opt(&data.default_theme); - let legal_information = sanitize_html_opt(&data.legal_information); + let application_question = sanitize_html_api_opt(&data.application_question); + let default_theme = sanitize_html_api_opt(&data.default_theme); + let legal_information = sanitize_html_api_opt(&data.legal_information); let local_site_form = LocalSiteUpdateForm { // Set the site setup to true diff --git a/crates/api_crud/src/site/update.rs b/crates/api_crud/src/site/update.rs index 6148d546e..4d1fbd4d9 100644 --- a/crates/api_crud/src/site/update.rs +++ b/crates/api_crud/src/site/update.rs @@ -7,7 +7,7 @@ use lemmy_api_common::{ is_admin, local_site_rate_limit_to_rate_limit_config, local_user_view_from_jwt, - sanitize_html_opt, + sanitize_html_api_opt, }, }; use lemmy_db_schema::{ @@ -59,9 +59,9 @@ pub async fn update_site( SiteLanguage::update(&mut context.pool(), discussion_languages.clone(), &site).await?; } - let name = sanitize_html_opt(&data.name); - let sidebar = sanitize_html_opt(&data.sidebar); - let description = sanitize_html_opt(&data.description); + let name = sanitize_html_api_opt(&data.name); + let sidebar = sanitize_html_api_opt(&data.sidebar); + let description = sanitize_html_api_opt(&data.description); let site_form = SiteUpdateForm { name, @@ -79,9 +79,9 @@ pub async fn update_site( // Diesel will throw an error for empty update forms .ok(); - let application_question = sanitize_html_opt(&data.application_question); - let default_theme = sanitize_html_opt(&data.default_theme); - let legal_information = sanitize_html_opt(&data.legal_information); + let application_question = sanitize_html_api_opt(&data.application_question); + let default_theme = sanitize_html_api_opt(&data.default_theme); + let legal_information = sanitize_html_api_opt(&data.legal_information); let local_site_form = LocalSiteUpdateForm { enable_downvotes: data.enable_downvotes, diff --git a/crates/api_crud/src/user/create.rs b/crates/api_crud/src/user/create.rs index d855434ec..02c95cb04 100644 --- a/crates/api_crud/src/user/create.rs +++ b/crates/api_crud/src/user/create.rs @@ -10,7 +10,7 @@ use lemmy_api_common::{ honeypot_check, local_site_to_slur_regex, password_length_check, - sanitize_html, + sanitize_html_api, send_new_applicant_email_to_admins, send_verification_email, EndpointType, @@ -89,7 +89,7 @@ pub async fn register( let slur_regex = local_site_to_slur_regex(&local_site); check_slurs(&data.username, &slur_regex)?; check_slurs_opt(&data.answer, &slur_regex)?; - let username = sanitize_html(&data.username); + let username = sanitize_html_api(&data.username); let actor_keypair = generate_actor_keypair()?; is_valid_actor_name(&data.username, local_site.actor_name_max_length as usize)?; diff --git a/crates/apub/src/activities/block/block_user.rs b/crates/apub/src/activities/block/block_user.rs index 7b14213ca..07fe0b75b 100644 --- a/crates/apub/src/activities/block/block_user.rs +++ b/crates/apub/src/activities/block/block_user.rs @@ -23,7 +23,7 @@ use anyhow::anyhow; use chrono::{DateTime, Utc}; use lemmy_api_common::{ context::LemmyContext, - utils::{remove_user_data, remove_user_data_in_community, sanitize_html_opt}, + utils::{remove_user_data, remove_user_data_in_community, sanitize_html_federation_opt}, }; use lemmy_db_schema::{ source::{ @@ -172,7 +172,7 @@ impl ActivityHandler for BlockUser { let form = ModBanForm { mod_person_id: mod_person.id, other_person_id: blocked_person.id, - reason: sanitize_html_opt(&self.summary), + reason: sanitize_html_federation_opt(&self.summary), banned: Some(true), expires, }; @@ -206,7 +206,7 @@ impl ActivityHandler for BlockUser { mod_person_id: mod_person.id, other_person_id: blocked_person.id, community_id: community.id, - reason: sanitize_html_opt(&self.summary), + reason: sanitize_html_federation_opt(&self.summary), banned: Some(true), expires, }; diff --git a/crates/apub/src/activities/block/undo_block_user.rs b/crates/apub/src/activities/block/undo_block_user.rs index 5f6bf84cb..889d6a286 100644 --- a/crates/apub/src/activities/block/undo_block_user.rs +++ b/crates/apub/src/activities/block/undo_block_user.rs @@ -17,7 +17,7 @@ use activitypub_federation::{ protocol::verification::verify_domains_match, traits::{ActivityHandler, Actor}, }; -use lemmy_api_common::{context::LemmyContext, utils::sanitize_html_opt}; +use lemmy_api_common::{context::LemmyContext, utils::sanitize_html_federation_opt}; use lemmy_db_schema::{ source::{ community::{CommunityPersonBan, CommunityPersonBanForm}, @@ -117,7 +117,7 @@ impl ActivityHandler for UndoBlockUser { let form = ModBanForm { mod_person_id: mod_person.id, other_person_id: blocked_person.id, - reason: sanitize_html_opt(&self.object.summary), + reason: sanitize_html_federation_opt(&self.object.summary), banned: Some(false), expires, }; @@ -136,7 +136,7 @@ impl ActivityHandler for UndoBlockUser { mod_person_id: mod_person.id, other_person_id: blocked_person.id, community_id: community.id, - reason: sanitize_html_opt(&self.object.summary), + reason: sanitize_html_federation_opt(&self.object.summary), banned: Some(false), expires, }; diff --git a/crates/apub/src/activities/community/report.rs b/crates/apub/src/activities/community/report.rs index a17df7119..16f8c3bdf 100644 --- a/crates/apub/src/activities/community/report.rs +++ b/crates/apub/src/activities/community/report.rs @@ -11,7 +11,7 @@ use activitypub_federation::{ kinds::activity::FlagType, traits::{ActivityHandler, Actor}, }; -use lemmy_api_common::{context::LemmyContext, utils::sanitize_html}; +use lemmy_api_common::{context::LemmyContext, utils::sanitize_html_federation}; use lemmy_db_schema::{ source::{ comment_report::{CommentReport, CommentReportForm}, @@ -86,7 +86,7 @@ impl ActivityHandler for Report { post_id: post.id, original_post_name: post.name.clone(), original_post_url: post.url.clone(), - reason: sanitize_html(&self.summary), + reason: sanitize_html_federation(&self.summary), original_post_body: post.body.clone(), }; PostReport::report(&mut context.pool(), &report_form).await?; @@ -96,7 +96,7 @@ impl ActivityHandler for Report { creator_id: actor.id, comment_id: comment.id, original_comment_text: comment.content.clone(), - reason: sanitize_html(&self.summary), + reason: sanitize_html_federation(&self.summary), }; CommentReport::report(&mut context.pool(), &report_form).await?; } diff --git a/crates/apub/src/activities/deletion/delete.rs b/crates/apub/src/activities/deletion/delete.rs index af289640b..88bf2b523 100644 --- a/crates/apub/src/activities/deletion/delete.rs +++ b/crates/apub/src/activities/deletion/delete.rs @@ -8,7 +8,7 @@ use crate::{ protocol::{activities::deletion::delete::Delete, IdOrNestedObject}, }; use activitypub_federation::{config::Data, kinds::activity::DeleteType, traits::ActivityHandler}; -use lemmy_api_common::{context::LemmyContext, utils::sanitize_html_opt}; +use lemmy_api_common::{context::LemmyContext, utils::sanitize_html_federation_opt}; use lemmy_db_schema::{ source::{ comment::{Comment, CommentUpdateForm}, @@ -105,7 +105,7 @@ pub(in crate::activities) async fn receive_remove_action( reason: Option, context: &Data, ) -> Result<(), LemmyError> { - let reason = sanitize_html_opt(&reason); + let reason = sanitize_html_federation_opt(&reason); match DeletableObjects::read_from_db(object, context).await? { DeletableObjects::Community(community) => { diff --git a/crates/apub/src/objects/comment.rs b/crates/apub/src/objects/comment.rs index 888e0fffb..599dd7387 100644 --- a/crates/apub/src/objects/comment.rs +++ b/crates/apub/src/objects/comment.rs @@ -18,7 +18,7 @@ use activitypub_federation::{ use chrono::{DateTime, Utc}; use lemmy_api_common::{ context::LemmyContext, - utils::{local_site_opt_to_slur_regex, sanitize_html}, + utils::{local_site_opt_to_slur_regex, sanitize_html_federation}, }; use lemmy_db_schema::{ source::{ @@ -162,7 +162,7 @@ impl Object for ApubComment { let local_site = LocalSite::read(&mut context.pool()).await.ok(); let slur_regex = &local_site_opt_to_slur_regex(&local_site); let content = remove_slurs(&content, slur_regex); - let content = sanitize_html(&content); + let content = sanitize_html_federation(&content); let language_id = LanguageTag::to_language_id_single(note.language, &mut context.pool()).await?; diff --git a/crates/apub/src/objects/instance.rs b/crates/apub/src/objects/instance.rs index 5cbe75eda..72c21e518 100644 --- a/crates/apub/src/objects/instance.rs +++ b/crates/apub/src/objects/instance.rs @@ -18,7 +18,7 @@ use activitypub_federation::{ use chrono::{DateTime, Utc}; use lemmy_api_common::{ context::LemmyContext, - utils::{local_site_opt_to_slur_regex, sanitize_html_opt}, + utils::{local_site_opt_to_slur_regex, sanitize_html_federation_opt}, }; use lemmy_db_schema::{ newtypes::InstanceId, @@ -133,8 +133,8 @@ impl Object for ApubSite { let instance = DbInstance::read_or_create(&mut data.pool(), domain.to_string()).await?; let sidebar = read_from_string_or_source_opt(&apub.content, &None, &apub.source); - let sidebar = sanitize_html_opt(&sidebar); - let description = sanitize_html_opt(&apub.summary); + let sidebar = sanitize_html_federation_opt(&sidebar); + let description = sanitize_html_federation_opt(&apub.summary); let site_form = SiteInsertForm { name: apub.name.clone(), diff --git a/crates/apub/src/objects/person.rs b/crates/apub/src/objects/person.rs index a0e7f590a..c5468e432 100644 --- a/crates/apub/src/objects/person.rs +++ b/crates/apub/src/objects/person.rs @@ -19,7 +19,12 @@ use activitypub_federation::{ use chrono::{DateTime, Utc}; use lemmy_api_common::{ context::LemmyContext, - utils::{generate_outbox_url, local_site_opt_to_slur_regex, sanitize_html, sanitize_html_opt}, + utils::{ + generate_outbox_url, + local_site_opt_to_slur_regex, + sanitize_html_federation, + sanitize_html_federation_opt, + }, }; use lemmy_db_schema::{ source::person::{Person as DbPerson, PersonInsertForm, PersonUpdateForm}, @@ -141,10 +146,10 @@ impl Object for ApubPerson { ) -> Result { let instance_id = fetch_instance_actor_for_object(&person.id, context).await?; - let name = sanitize_html(&person.preferred_username); - let display_name = sanitize_html_opt(&person.name); + let name = sanitize_html_federation(&person.preferred_username); + let display_name = sanitize_html_federation_opt(&person.name); let bio = read_from_string_or_source_opt(&person.summary, &None, &person.source); - let bio = sanitize_html_opt(&bio); + let bio = sanitize_html_federation_opt(&bio); // Some Mastodon users have `name: ""` (empty string), need to convert that to `None` // https://github.com/mastodon/mastodon/issues/25233 @@ -260,7 +265,7 @@ pub(crate) mod tests { assert_eq!(person.name, "lanodan"); assert!(!person.local); assert_eq!(context.request_count(), 0); - assert_eq!(person.bio.as_ref().unwrap().len(), 873); + assert_eq!(person.bio.as_ref().unwrap().len(), 878); cleanup((person, site), &context).await; } diff --git a/crates/apub/src/objects/post.rs b/crates/apub/src/objects/post.rs index aee2eaf90..29867fced 100644 --- a/crates/apub/src/objects/post.rs +++ b/crates/apub/src/objects/post.rs @@ -29,8 +29,8 @@ use lemmy_api_common::{ is_mod_or_admin, local_site_opt_to_sensitive, local_site_opt_to_slur_regex, - sanitize_html, - sanitize_html_opt, + sanitize_html_federation, + sanitize_html_federation_opt, }, }; use lemmy_db_schema::{ @@ -237,9 +237,9 @@ impl Object for ApubPost { let language_id = LanguageTag::to_language_id_single(page.language, &mut context.pool()).await?; - let name = sanitize_html(&name); - let embed_title = sanitize_html_opt(&embed_title); - let embed_description = sanitize_html_opt(&embed_description); + let name = sanitize_html_federation(&name); + let embed_title = sanitize_html_federation_opt(&embed_title); + let embed_description = sanitize_html_federation_opt(&embed_description); PostInsertForm { name, diff --git a/crates/apub/src/objects/private_message.rs b/crates/apub/src/objects/private_message.rs index 3d637d624..97bf595d5 100644 --- a/crates/apub/src/objects/private_message.rs +++ b/crates/apub/src/objects/private_message.rs @@ -14,7 +14,7 @@ use activitypub_federation::{ use chrono::{DateTime, Utc}; use lemmy_api_common::{ context::LemmyContext, - utils::{check_person_block, sanitize_html}, + utils::{check_person_block, sanitize_html_federation}, }; use lemmy_db_schema::{ source::{ @@ -125,7 +125,7 @@ impl Object for ApubPrivateMessage { check_person_block(creator.id, recipient.id, &mut context.pool()).await?; let content = read_from_string_or_source(¬e.content, &None, ¬e.source); - let content = sanitize_html(&content); + let content = sanitize_html_federation(&content); let form = PrivateMessageInsertForm { creator_id: creator.id, diff --git a/crates/apub/src/protocol/objects/group.rs b/crates/apub/src/protocol/objects/group.rs index 3ee788f94..9d0229785 100644 --- a/crates/apub/src/protocol/objects/group.rs +++ b/crates/apub/src/protocol/objects/group.rs @@ -25,7 +25,7 @@ use activitypub_federation::{ use chrono::{DateTime, Utc}; use lemmy_api_common::{ context::LemmyContext, - utils::{local_site_opt_to_slur_regex, sanitize_html, sanitize_html_opt}, + utils::{local_site_opt_to_slur_regex, sanitize_html_federation, sanitize_html_federation_opt}, }; use lemmy_db_schema::{ newtypes::InstanceId, @@ -97,10 +97,10 @@ impl Group { } pub(crate) fn into_insert_form(self, instance_id: InstanceId) -> CommunityInsertForm { - let name = sanitize_html(&self.preferred_username); - let title = sanitize_html(&self.name.unwrap_or(self.preferred_username)); + let name = sanitize_html_federation(&self.preferred_username); + let title = sanitize_html_federation(&self.name.unwrap_or(self.preferred_username)); let description = read_from_string_or_source_opt(&self.summary, &None, &self.source); - let description = sanitize_html_opt(&description); + let description = sanitize_html_federation_opt(&description); CommunityInsertForm { name, diff --git a/crates/db_schema/src/schema.rs b/crates/db_schema/src/schema.rs index e5901200f..ffcdbebe8 100644 --- a/crates/db_schema/src/schema.rs +++ b/crates/db_schema/src/schema.rs @@ -299,16 +299,6 @@ diesel::table! { } } -diesel::table! { - image_upload (id) { - id -> Int4, - local_user_id -> Int4, - pictrs_alias -> Text, - pictrs_delete_token -> Text, - published -> Timestamptz, - } -} - diesel::table! { instance (id) { id -> Int4, @@ -415,9 +405,9 @@ diesel::table! { totp_2fa_secret -> Nullable, totp_2fa_url -> Nullable, open_links_in_new_tab -> Bool, - infinite_scroll_enabled -> Bool, blur_nsfw -> Bool, auto_expand -> Bool, + infinite_scroll_enabled -> Bool, admin -> Bool, post_listing_mode -> PostListingModeEnum, } @@ -903,7 +893,6 @@ diesel::joinable!(custom_emoji_keyword -> custom_emoji (custom_emoji_id)); diesel::joinable!(email_verification -> local_user (local_user_id)); diesel::joinable!(federation_allowlist -> instance (instance_id)); diesel::joinable!(federation_blocklist -> instance (instance_id)); -diesel::joinable!(image_upload -> local_user (local_user_id)); diesel::joinable!(local_site -> site (site_id)); diesel::joinable!(local_site_rate_limit -> local_site (local_site_id)); diesel::joinable!(local_user -> person (person_id)); @@ -978,7 +967,6 @@ diesel::allow_tables_to_appear_in_same_query!( email_verification, federation_allowlist, federation_blocklist, - image_upload, instance, language, local_site,