Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sanitize html #3708

Merged
merged 6 commits into from
Jul 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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
Original file line number Diff line number Diff line change
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 @@ -504,3 +505,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
Original file line number Diff line number Diff line change
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(&mut 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(&mut context.pool(), &report_form)
Expand Down
9 changes: 7 additions & 2 deletions crates/api/src/community/ban.rs
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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(&mut 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 @@ -85,6 +88,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 @@ -130,7 +134,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
Original file line number Diff line number Diff line change
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(&mut 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(&mut context.pool(), &report_form)
Expand Down
8 changes: 4 additions & 4 deletions crates/api/src/private_message_report/create.rs
Original file line number Diff line number Diff line change
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(&mut 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(&mut context.pool(), &report_form)
Expand Down
4 changes: 2 additions & 2 deletions crates/api/src/site/purge/comment.rs
Original file line number Diff line number Diff line change
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(&mut 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
Original file line number Diff line number Diff line change
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(&mut 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
Original file line number Diff line number Diff line change
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(&mut 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
Original file line number Diff line number Diff line change
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(&mut 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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ full = [
"actix-web",
"futures",
"once_cell",
"ammonia",
]

[dependencies]
Expand Down Expand Up @@ -66,3 +67,4 @@ 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 }