From cc2c7db9fec3cf86b5dd488f3c48d556d80ed529 Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Thu, 6 Aug 2020 14:53:58 +0200 Subject: [PATCH] Add security checks and slur checks for activitypub inbox --- server/lemmy_db/src/comment.rs | 10 +--- server/lemmy_db/src/community.rs | 10 +--- server/lemmy_db/src/post.rs | 10 +--- server/src/api/mod.rs | 5 +- server/src/apub/comment.rs | 23 ++++----- server/src/apub/community.rs | 52 ++++++++++++-------- server/src/apub/fetcher.rs | 21 ++++---- server/src/apub/inbox/activities/announce.rs | 34 ++++++++----- server/src/apub/inbox/activities/create.rs | 20 +++++++- server/src/apub/inbox/activities/delete.rs | 7 +-- server/src/apub/inbox/activities/dislike.rs | 4 +- server/src/apub/inbox/activities/like.rs | 4 +- server/src/apub/inbox/activities/remove.rs | 20 ++++++-- server/src/apub/inbox/activities/undo.rs | 48 ++++++++++++++---- server/src/apub/inbox/activities/update.rs | 52 +++++++++++++++----- server/src/apub/inbox/shared_inbox.rs | 16 ++++-- server/src/apub/inbox/user_inbox.rs | 12 +++-- server/src/apub/mod.rs | 29 +++++++++++ server/src/apub/post.rs | 28 ++++++----- server/src/apub/private_message.rs | 4 +- server/src/apub/user.rs | 48 ++++++++++-------- 21 files changed, 299 insertions(+), 158 deletions(-) diff --git a/server/lemmy_db/src/comment.rs b/server/lemmy_db/src/comment.rs index 99efde8d7..cdb5a1b64 100644 --- a/server/lemmy_db/src/comment.rs +++ b/server/lemmy_db/src/comment.rs @@ -116,10 +116,7 @@ impl Comment { ) -> Result { use crate::schema::comment::dsl::*; diesel::update(comment.find(comment_id)) - .set(( - deleted.eq(new_deleted), - updated.eq(naive_now()) - )) + .set((deleted.eq(new_deleted), updated.eq(naive_now()))) .get_result::(conn) } @@ -130,10 +127,7 @@ impl Comment { ) -> Result { use crate::schema::comment::dsl::*; diesel::update(comment.find(comment_id)) - .set(( - removed.eq(new_removed), - updated.eq(naive_now()) - )) + .set((removed.eq(new_removed), updated.eq(naive_now()))) .get_result::(conn) } diff --git a/server/lemmy_db/src/community.rs b/server/lemmy_db/src/community.rs index 2c86f1e75..14e8f9849 100644 --- a/server/lemmy_db/src/community.rs +++ b/server/lemmy_db/src/community.rs @@ -107,10 +107,7 @@ impl Community { ) -> Result { use crate::schema::community::dsl::*; diesel::update(community.find(community_id)) - .set(( - deleted.eq(new_deleted), - updated.eq(naive_now()) - )) + .set((deleted.eq(new_deleted), updated.eq(naive_now()))) .get_result::(conn) } @@ -121,10 +118,7 @@ impl Community { ) -> Result { use crate::schema::community::dsl::*; diesel::update(community.find(community_id)) - .set(( - removed.eq(new_removed), - updated.eq(naive_now()) - )) + .set((removed.eq(new_removed), updated.eq(naive_now()))) .get_result::(conn) } diff --git a/server/lemmy_db/src/post.rs b/server/lemmy_db/src/post.rs index 56ff7474b..43e002113 100644 --- a/server/lemmy_db/src/post.rs +++ b/server/lemmy_db/src/post.rs @@ -119,10 +119,7 @@ impl Post { ) -> Result { use crate::schema::post::dsl::*; diesel::update(post.find(post_id)) - .set(( - deleted.eq(new_deleted), - updated.eq(naive_now()) - )) + .set((deleted.eq(new_deleted), updated.eq(naive_now()))) .get_result::(conn) } @@ -133,10 +130,7 @@ impl Post { ) -> Result { use crate::schema::post::dsl::*; diesel::update(post.find(post_id)) - .set(( - removed.eq(new_removed), - updated.eq(naive_now()) - )) + .set((removed.eq(new_removed), updated.eq(naive_now()))) .get_result::(conn) } diff --git a/server/src/api/mod.rs b/server/src/api/mod.rs index a9aae823a..7c5eeb2fa 100644 --- a/server/src/api/mod.rs +++ b/server/src/api/mod.rs @@ -65,6 +65,7 @@ pub async fn is_mod_or_admin( }) .await?; if !is_mod_or_admin { + // TODO: more accurately, not_a_mod_or_admin? return Err(APIError::err("not_an_admin").into()); } Ok(()) @@ -104,14 +105,14 @@ pub(in crate::api) async fn get_user_from_jwt_opt( } } -pub(in crate::api) fn check_slurs(text: &str) -> Result<(), APIError> { +pub(in crate) fn check_slurs(text: &str) -> Result<(), APIError> { if let Err(slurs) = slur_check(text) { Err(APIError::err(&slurs_vec_to_str(slurs))) } else { Ok(()) } } -pub(in crate::api) fn check_slurs_opt(text: &Option) -> Result<(), APIError> { +pub(in crate) fn check_slurs_opt(text: &Option) -> Result<(), APIError> { match text { Some(t) => check_slurs(t), None => Ok(()), diff --git a/server/src/apub/comment.rs b/server/src/apub/comment.rs index 8bd79b799..1aa3790cc 100644 --- a/server/src/apub/comment.rs +++ b/server/src/apub/comment.rs @@ -1,7 +1,8 @@ use crate::{ + api::check_slurs, apub::{ activities::{generate_activity_id, send_activity_to_community}, - check_is_apub_id_valid, + check_actor_domain, create_apub_response, create_apub_tombstone_response, create_tombstone, @@ -132,6 +133,7 @@ impl FromApub for CommentForm { note: &Note, client: &Client, pool: &DbPool, + expected_domain: Option, ) -> Result { let creator_actor_id = ¬e .attributed_to() @@ -166,26 +168,25 @@ impl FromApub for CommentForm { } None => None, }; - - let ap_id = note.id_unchecked().unwrap().to_string(); - check_is_apub_id_valid(&Url::parse(&ap_id)?)?; + let content = note + .content() + .unwrap() + .as_single_xsd_string() + .unwrap() + .to_string(); + check_slurs(&content)?; Ok(CommentForm { creator_id: creator.id, post_id: post.id, parent_id, - content: note - .content() - .unwrap() - .as_single_xsd_string() - .unwrap() - .to_string(), + content, removed: None, read: None, published: note.published().map(|u| u.to_owned().naive_local()), updated: note.updated().map(|u| u.to_owned().naive_local()), deleted: None, - ap_id, + ap_id: check_actor_domain(note, expected_domain)?, local: false, }) } diff --git a/server/src/apub/community.rs b/server/src/apub/community.rs index 44e3ad4e6..3773b8fb4 100644 --- a/server/src/apub/community.rs +++ b/server/src/apub/community.rs @@ -1,7 +1,8 @@ use crate::{ + api::{check_slurs, check_slurs_opt}, apub::{ activities::{generate_activity_id, send_activity}, - check_is_apub_id_valid, + check_actor_domain, create_apub_response, create_apub_tombstone_response, create_tombstone, @@ -323,7 +324,12 @@ impl FromApub for CommunityForm { type ApubType = GroupExt; /// Parse an ActivityPub group received from another instance into a Lemmy community. - async fn from_apub(group: &GroupExt, client: &Client, pool: &DbPool) -> Result { + async fn from_apub( + group: &GroupExt, + client: &Client, + pool: &DbPool, + expected_domain: Option, + ) -> Result { let creator_and_moderator_uris = group.inner.attributed_to().unwrap(); let creator_uri = creator_and_moderator_uris .as_many() @@ -335,26 +341,30 @@ impl FromApub for CommunityForm { .unwrap(); let creator = get_or_fetch_and_upsert_user(creator_uri, client, pool).await?; - let actor_id = group.inner.id_unchecked().unwrap().to_string(); - check_is_apub_id_valid(&Url::parse(&actor_id)?)?; + let name = group + .inner + .name() + .unwrap() + .as_one() + .unwrap() + .as_xsd_string() + .unwrap() + .to_string(); + let title = group.inner.preferred_username().unwrap().to_string(); + // TODO: should be parsed as html and tags like