Skip to content

Commit

Permalink
Sanitize html (#3708)
Browse files Browse the repository at this point in the history
* HTML sanitization in apub code

* Sanitize API inputs

* fmt

* Dont allow html a, img tags

---------

Co-authored-by: Dessalines <dessalines@users.noreply.github.com>
  • Loading branch information
Nutomic and dessalines committed Jul 27, 2023
1 parent c5450e7 commit 8468cb4
Show file tree
Hide file tree
Showing 42 changed files with 340 additions and 151 deletions.
20 changes: 20 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

19 changes: 19 additions & 0 deletions api_tests/src/post.spec.ts
Expand Up @@ -36,6 +36,7 @@ import {
resolveCommunity,
} from "./shared";
import { PostView } from "lemmy-js-client/dist/types/PostView";
import { CreatePost } from "lemmy-js-client/dist/types/CreatePost";

let betaCommunity: CommunityView | undefined;

Expand Down Expand Up @@ -499,3 +500,21 @@ test("Report a post", async () => {
expect(betaReport.original_post_body).toBe(alphaReport.original_post_body);
expect(betaReport.reason).toBe(alphaReport.reason);
});

test("Sanitize HTML", async () => {
let betaCommunity = (await resolveBetaCommunity(beta)).community;
if (!betaCommunity) {
throw "Missing beta community";
}

let name = randomString(5);
let body = "<script>alert('xss');</script> hello";
let form: CreatePost = {
name,
body,
auth: beta.auth,
community_id: betaCommunity.community.id,
};
let post = await beta.client.createPost(form);
expect(post.post_view.post.body).toBe(" hello");
});
13 changes: 9 additions & 4 deletions crates/api/src/comment_report/create.rs
Expand Up @@ -3,7 +3,12 @@ use actix_web::web::Data;
use lemmy_api_common::{
comment::{CommentReportResponse, CreateCommentReport},
context::LemmyContext,
utils::{check_community_ban, local_user_view_from_jwt, send_new_report_email_to_admins},
utils::{
check_community_ban,
local_user_view_from_jwt,
sanitize_html,
send_new_report_email_to_admins,
},
};
use lemmy_db_schema::{
source::{
Expand All @@ -29,8 +34,8 @@ impl Perform for CreateCommentReport {
let local_user_view = local_user_view_from_jwt(&data.auth, context).await?;
let local_site = LocalSite::read(context.pool()).await?;

let reason = self.reason.trim();
check_report_reason(reason, &local_site)?;
let reason = sanitize_html(self.reason.trim());
check_report_reason(&reason, &local_site)?;

let person_id = local_user_view.person.id;
let comment_id = data.comment_id;
Expand All @@ -42,7 +47,7 @@ impl Perform for CreateCommentReport {
creator_id: person_id,
comment_id,
original_comment_text: comment_view.comment.content,
reason: reason.to_owned(),
reason,
};

let report = CommentReport::report(context.pool(), &report_form)
Expand Down
9 changes: 7 additions & 2 deletions crates/api/src/community/ban.rs
Expand Up @@ -3,7 +3,12 @@ use actix_web::web::Data;
use lemmy_api_common::{
community::{BanFromCommunity, BanFromCommunityResponse},
context::LemmyContext,
utils::{is_mod_or_admin, local_user_view_from_jwt, remove_user_data_in_community},
utils::{
is_mod_or_admin,
local_user_view_from_jwt,
remove_user_data_in_community,
sanitize_html_opt,
},
};
use lemmy_db_schema::{
source::{
Expand Down Expand Up @@ -81,7 +86,7 @@ impl Perform for BanFromCommunity {
mod_person_id: local_user_view.person.id,
other_person_id: data.person_id,
community_id: data.community_id,
reason: data.reason.clone(),
reason: sanitize_html_opt(&data.reason),
banned: Some(data.ban),
expires,
};
Expand Down
4 changes: 2 additions & 2 deletions crates/api/src/community/hide.rs
Expand Up @@ -4,7 +4,7 @@ use lemmy_api_common::{
build_response::build_community_response,
community::{CommunityResponse, HideCommunity},
context::LemmyContext,
utils::{is_admin, local_user_view_from_jwt},
utils::{is_admin, local_user_view_from_jwt, sanitize_html_opt},
};
use lemmy_db_schema::{
source::{
Expand Down Expand Up @@ -34,7 +34,7 @@ impl Perform for HideCommunity {
let mod_hide_community_form = ModHideCommunityForm {
community_id: data.community_id,
mod_person_id: local_user_view.person.id,
reason: data.reason.clone(),
reason: sanitize_html_opt(&data.reason),
hidden: Some(data.hidden),
};

Expand Down
2 changes: 1 addition & 1 deletion crates/api/src/lib.rs
Expand Up @@ -62,7 +62,7 @@ pub(crate) fn captcha_as_wav_base64(captcha: &Captcha) -> Result<String, LemmyEr
Ok(base64.encode(output_buffer.into_inner()))
}

/// Check size of report and remove whitespace
/// Check size of report
pub(crate) fn check_report_reason(reason: &str, local_site: &LocalSite) -> Result<(), LemmyError> {
let slur_regex = &local_site_to_slur_regex(local_site);

Expand Down
4 changes: 2 additions & 2 deletions crates/api/src/local_user/ban_person.rs
Expand Up @@ -3,7 +3,7 @@ use actix_web::web::Data;
use lemmy_api_common::{
context::LemmyContext,
person::{BanPerson, BanPersonResponse},
utils::{is_admin, local_user_view_from_jwt, remove_user_data},
utils::{is_admin, local_user_view_from_jwt, remove_user_data, sanitize_html_opt},
};
use lemmy_db_schema::{
source::{
Expand Down Expand Up @@ -63,7 +63,7 @@ impl Perform for BanPerson {
let form = ModBanForm {
mod_person_id: local_user_view.person.id,
other_person_id: data.person_id,
reason: data.reason.clone(),
reason: sanitize_html_opt(&data.reason),
banned: Some(data.ban),
expires,
};
Expand Down
16 changes: 10 additions & 6 deletions crates/api/src/local_user/save_settings.rs
Expand Up @@ -3,7 +3,7 @@ use actix_web::web::Data;
use lemmy_api_common::{
context::LemmyContext,
person::{LoginResponse, SaveUserSettings},
utils::{local_user_view_from_jwt, send_verification_email},
utils::{local_user_view_from_jwt, sanitize_html_opt, send_verification_email},
};
use lemmy_db_schema::{
source::{
Expand Down Expand Up @@ -37,13 +37,16 @@ impl Perform for SaveUserSettings {
let local_user_view = local_user_view_from_jwt(&data.auth, context).await?;
let site_view = SiteView::read_local(context.pool()).await?;

let bio = sanitize_html_opt(&data.bio);
let display_name = sanitize_html_opt(&data.display_name);

let avatar = diesel_option_overwrite_to_url(&data.avatar)?;
let banner = diesel_option_overwrite_to_url(&data.banner)?;
let bio = diesel_option_overwrite(&data.bio);
let display_name = diesel_option_overwrite(&data.display_name);
let matrix_user_id = diesel_option_overwrite(&data.matrix_user_id);
let bio = diesel_option_overwrite(bio);
let display_name = diesel_option_overwrite(display_name);
let matrix_user_id = diesel_option_overwrite(data.matrix_user_id.clone());
let email_deref = data.email.as_deref().map(str::to_lowercase);
let email = diesel_option_overwrite(&email_deref);
let email = diesel_option_overwrite(email_deref.clone());

if let Some(Some(email)) = &email {
let previous_email = local_user_view.local_user.email.clone().unwrap_or_default();
Expand Down Expand Up @@ -80,6 +83,7 @@ impl Perform for SaveUserSettings {
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 person_form = PersonUpdateForm::builder()
.display_name(display_name)
Expand Down Expand Up @@ -123,7 +127,7 @@ impl Perform for SaveUserSettings {
.show_scores(data.show_scores)
.default_sort_type(default_sort_type)
.default_listing_type(default_listing_type)
.theme(data.theme.clone())
.theme(theme)
.interface_language(data.interface_language.clone())
.totp_2fa_secret(totp_2fa_secret)
.totp_2fa_url(totp_2fa_url)
Expand Down
13 changes: 9 additions & 4 deletions crates/api/src/post_report/create.rs
Expand Up @@ -3,7 +3,12 @@ use actix_web::web::Data;
use lemmy_api_common::{
context::LemmyContext,
post::{CreatePostReport, PostReportResponse},
utils::{check_community_ban, local_user_view_from_jwt, send_new_report_email_to_admins},
utils::{
check_community_ban,
local_user_view_from_jwt,
sanitize_html,
send_new_report_email_to_admins,
},
};
use lemmy_db_schema::{
source::{
Expand All @@ -26,8 +31,8 @@ impl Perform for CreatePostReport {
let local_user_view = local_user_view_from_jwt(&data.auth, context).await?;
let local_site = LocalSite::read(context.pool()).await?;

let reason = self.reason.trim();
check_report_reason(reason, &local_site)?;
let reason = sanitize_html(self.reason.trim());
check_report_reason(&reason, &local_site)?;

let person_id = local_user_view.person.id;
let post_id = data.post_id;
Expand All @@ -41,7 +46,7 @@ impl Perform for CreatePostReport {
original_post_name: post_view.post.name,
original_post_url: post_view.post.url,
original_post_body: post_view.post.body,
reason: reason.to_owned(),
reason,
};

let report = PostReport::report(context.pool(), &report_form)
Expand Down
8 changes: 4 additions & 4 deletions crates/api/src/private_message_report/create.rs
Expand Up @@ -3,7 +3,7 @@ use actix_web::web::Data;
use lemmy_api_common::{
context::LemmyContext,
private_message::{CreatePrivateMessageReport, PrivateMessageReportResponse},
utils::{local_user_view_from_jwt, send_new_report_email_to_admins},
utils::{local_user_view_from_jwt, sanitize_html, send_new_report_email_to_admins},
};
use lemmy_db_schema::{
source::{
Expand All @@ -25,8 +25,8 @@ impl Perform for CreatePrivateMessageReport {
let local_user_view = local_user_view_from_jwt(&self.auth, context).await?;
let local_site = LocalSite::read(context.pool()).await?;

let reason = self.reason.trim();
check_report_reason(reason, &local_site)?;
let reason = sanitize_html(self.reason.trim());
check_report_reason(&reason, &local_site)?;

let person_id = local_user_view.person.id;
let private_message_id = self.private_message_id;
Expand All @@ -36,7 +36,7 @@ impl Perform for CreatePrivateMessageReport {
creator_id: person_id,
private_message_id,
original_pm_text: private_message.content,
reason: reason.to_owned(),
reason: reason.clone(),
};

let report = PrivateMessageReport::report(context.pool(), &report_form)
Expand Down
4 changes: 2 additions & 2 deletions crates/api/src/site/purge/comment.rs
Expand Up @@ -3,7 +3,7 @@ use actix_web::web::Data;
use lemmy_api_common::{
context::LemmyContext,
site::{PurgeComment, PurgeItemResponse},
utils::{is_admin, local_user_view_from_jwt},
utils::{is_admin, local_user_view_from_jwt, sanitize_html_opt},
};
use lemmy_db_schema::{
source::{
Expand Down Expand Up @@ -38,7 +38,7 @@ impl Perform for PurgeComment {
Comment::delete(context.pool(), comment_id).await?;

// Mod tables
let reason = data.reason.clone();
let reason = sanitize_html_opt(&data.reason);
let form = AdminPurgeCommentForm {
admin_person_id: local_user_view.person.id,
reason,
Expand Down
4 changes: 2 additions & 2 deletions crates/api/src/site/purge/community.rs
Expand Up @@ -4,7 +4,7 @@ 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},
utils::{is_admin, local_user_view_from_jwt, purge_image_posts_for_community, sanitize_html_opt},
};
use lemmy_db_schema::{
source::{
Expand Down Expand Up @@ -55,7 +55,7 @@ impl Perform for PurgeCommunity {
Community::delete(context.pool(), community_id).await?;

// Mod tables
let reason = data.reason.clone();
let reason = sanitize_html_opt(&data.reason);
let form = AdminPurgeCommunityForm {
admin_person_id: local_user_view.person.id,
reason,
Expand Down
4 changes: 2 additions & 2 deletions crates/api/src/site/purge/person.rs
Expand Up @@ -4,7 +4,7 @@ use lemmy_api_common::{
context::LemmyContext,
request::purge_image_from_pictrs,
site::{PurgeItemResponse, PurgePerson},
utils::{is_admin, local_user_view_from_jwt, purge_image_posts_for_person},
utils::{is_admin, local_user_view_from_jwt, purge_image_posts_for_person, sanitize_html_opt},
};
use lemmy_db_schema::{
source::{
Expand Down Expand Up @@ -54,7 +54,7 @@ impl Perform for PurgePerson {
Person::delete(context.pool(), person_id).await?;

// Mod tables
let reason = data.reason.clone();
let reason = sanitize_html_opt(&data.reason);
let form = AdminPurgePersonForm {
admin_person_id: local_user_view.person.id,
reason,
Expand Down
4 changes: 2 additions & 2 deletions crates/api/src/site/purge/post.rs
Expand Up @@ -4,7 +4,7 @@ use lemmy_api_common::{
context::LemmyContext,
request::purge_image_from_pictrs,
site::{PurgeItemResponse, PurgePost},
utils::{is_admin, local_user_view_from_jwt},
utils::{is_admin, local_user_view_from_jwt, sanitize_html_opt},
};
use lemmy_db_schema::{
source::{
Expand Down Expand Up @@ -50,7 +50,7 @@ impl Perform for PurgePost {
Post::delete(context.pool(), post_id).await?;

// Mod tables
let reason = data.reason.clone();
let reason = sanitize_html_opt(&data.reason);
let form = AdminPurgePostForm {
admin_person_id: local_user_view.person.id,
reason,
Expand Down
2 changes: 1 addition & 1 deletion crates/api/src/site/registration_applications/approve.rs
Expand Up @@ -30,7 +30,7 @@ impl Perform for ApproveRegistrationApplication {
is_admin(&local_user_view)?;

// Update the registration with reason, admin_id
let deny_reason = diesel_option_overwrite(&data.deny_reason);
let deny_reason = diesel_option_overwrite(data.deny_reason.clone());
let app_form = RegistrationApplicationUpdateForm {
admin_id: Some(Some(local_user_view.person.id)),
deny_reason,
Expand Down
2 changes: 2 additions & 0 deletions crates/api_common/Cargo.toml
Expand Up @@ -32,6 +32,7 @@ full = [
"reqwest",
"actix-web",
"futures",
"ammonia",
]

[dependencies]
Expand Down Expand Up @@ -61,3 +62,4 @@ reqwest = { workspace = true, optional = true }
ts-rs = { workspace = true, optional = true }
actix-web = { workspace = true, optional = true }
getrandom = { version = "0.2.10", features = ["js"] }
ammonia = { version = "3.3.0", optional = true }

0 comments on commit 8468cb4

Please sign in to comment.