Security/fix permission bugs (#966)

* secure the `EditPost` API endpoint

* Check user is moderator in BanFromCommunity

* secure the `EditComment` API endpoint

* pass orig `read` prob when not explicitly updating it.

* Block random users from adding mods.

* use cleaner logic from `EditPost`

* prevent editing a community by a mod from transfering ownership to them

* secure `read` action in `EditPrivateMessage`

* Add check in UserMention

* only let the indended recipient mark as read

* simplify booleans to satisfy clippy

* requested changes + cargo +nightly fmt

* fix to pass federation tests for deleting comments and posts

Co-authored-by: chiminh <chiminh.tutanota.com>
Co-authored-by: Hex Bear <buildadangtrain@protonmail.com>
This commit is contained in:
ryexandra 2020-07-14 07:17:25 -06:00 committed by GitHub
parent 52983907c4
commit 29037b4995
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
15 changed files with 381 additions and 175 deletions

View file

@ -181,7 +181,10 @@ mod tests {
pub fn establish_unpooled_connection() -> PgConnection {
let db_url = match get_database_url_from_env() {
Ok(url) => url,
Err(e) => panic!("Failed to read database URL from env var LEMMY_DATABASE_URL: {}", e),
Err(e) => panic!(
"Failed to read database URL from env var LEMMY_DATABASE_URL: {}",
e
),
};
PgConnection::establish(&db_url).unwrap_or_else(|_| panic!("Error connecting to {}", db_url))
}

View file

@ -1,7 +1,6 @@
use config::{Config, ConfigError, Environment, File};
use serde::Deserialize;
use std::{fs, io::Error, net::IpAddr, sync::RwLock};
use std::env;
use std::{env, fs, io::Error, net::IpAddr, sync::RwLock};
static CONFIG_FILE_DEFAULTS: &str = "config/defaults.hjson";
static CONFIG_FILE: &str = "config/config.hjson";

View file

@ -243,28 +243,28 @@ impl Perform for Oper<EditComment> {
let orig_comment =
blocking(pool, move |conn| CommentView::read(&conn, edit_id, None)).await??;
let mut editors: Vec<i32> = vec![orig_comment.creator_id];
let mut moderators: Vec<i32> = vec![];
let community_id = orig_comment.community_id;
moderators.append(
&mut blocking(pool, move |conn| {
CommunityModeratorView::for_community(&conn, community_id)
.map(|v| v.into_iter().map(|m| m.user_id).collect())
})
.await??,
);
moderators.append(
&mut blocking(pool, move |conn| {
UserView::admins(conn).map(|v| v.into_iter().map(|a| a.id).collect())
})
.await??,
);
editors.extend(&moderators);
// You are allowed to mark the comment as read even if you're banned.
if data.read.is_none() {
// Verify its the creator or a mod, or an admin
let mut editors: Vec<i32> = vec![data.creator_id];
let community_id = orig_comment.community_id;
editors.append(
&mut blocking(pool, move |conn| {
Ok(
CommunityModeratorView::for_community(&conn, community_id)?
.into_iter()
.map(|m| m.user_id)
.collect(),
) as Result<_, LemmyError>
})
.await??,
);
editors.append(
&mut blocking(pool, move |conn| {
Ok(UserView::admins(conn)?.into_iter().map(|a| a.id).collect()) as Result<_, LemmyError>
})
.await??,
);
if !editors.contains(&user_id) {
return Err(APIError::err("no_comment_edit_allowed").into());
@ -282,6 +282,25 @@ impl Perform for Oper<EditComment> {
if user.banned {
return Err(APIError::err("site_ban").into());
}
} else {
// check that user can mark as read
let parent_id = orig_comment.parent_id;
match parent_id {
Some(pid) => {
let parent_comment =
blocking(pool, move |conn| CommentView::read(&conn, pid, None)).await??;
if user_id != parent_comment.creator_id {
return Err(APIError::err("no_comment_edit_allowed").into());
}
}
None => {
let parent_post_id = orig_comment.post_id;
let parent_post = blocking(pool, move |conn| Post::read(conn, parent_post_id)).await??;
if user_id != parent_post.creator_id {
return Err(APIError::err("no_comment_edit_allowed").into());
}
}
}
}
let content_slurs_removed = remove_slurs(&data.content.to_owned());
@ -289,22 +308,45 @@ impl Perform for Oper<EditComment> {
let edit_id = data.edit_id;
let read_comment = blocking(pool, move |conn| Comment::read(conn, edit_id)).await??;
let comment_form = CommentForm {
content: content_slurs_removed,
parent_id: data.parent_id,
post_id: data.post_id,
creator_id: data.creator_id,
removed: data.removed.to_owned(),
deleted: data.deleted.to_owned(),
read: data.read.to_owned(),
published: None,
updated: if data.read.is_some() {
orig_comment.updated
let comment_form = {
if data.read.is_none() {
// the ban etc checks should been made and have passed
// the comment can be properly edited
let post_removed = if moderators.contains(&user_id) {
data.removed
} else {
Some(naive_now())
},
Some(read_comment.removed)
};
CommentForm {
content: content_slurs_removed,
parent_id: read_comment.parent_id,
post_id: read_comment.post_id,
creator_id: read_comment.creator_id,
removed: post_removed.to_owned(),
deleted: data.deleted.to_owned(),
read: Some(read_comment.read),
published: None,
updated: Some(naive_now()),
ap_id: read_comment.ap_id,
local: read_comment.local,
}
} else {
// the only field that can be updated it the read field
CommentForm {
content: read_comment.content,
parent_id: read_comment.parent_id,
post_id: read_comment.post_id,
creator_id: read_comment.creator_id,
removed: Some(read_comment.removed).to_owned(),
deleted: Some(read_comment.deleted).to_owned(),
read: data.read.to_owned(),
published: None,
updated: orig_comment.updated,
ap_id: read_comment.ap_id,
local: read_comment.local,
}
}
};
let edit_id = data.edit_id;
@ -318,6 +360,7 @@ impl Perform for Oper<EditComment> {
Err(_e) => return Err(APIError::err("couldnt_update_comment").into()),
};
if data.read.is_none() {
if let Some(deleted) = data.deleted.to_owned() {
if deleted {
updated_comment
@ -329,6 +372,7 @@ impl Perform for Oper<EditComment> {
.await?;
}
} else if let Some(removed) = data.removed.to_owned() {
if moderators.contains(&user_id) {
if removed {
updated_comment
.send_remove(&user, &self.client, pool)
@ -338,19 +382,15 @@ impl Perform for Oper<EditComment> {
.send_undo_remove(&user, &self.client, pool)
.await?;
}
}
} else {
updated_comment
.send_update(&user, &self.client, pool)
.await?;
}
let post_id = data.post_id;
let post = blocking(pool, move |conn| Post::read(conn, post_id)).await??;
let mentions = scrape_text_for_mentions(&comment_form.content);
let recipient_ids = send_local_notifs(mentions, updated_comment, user, post, pool).await?;
// Mod tables
if moderators.contains(&user_id) {
if let Some(removed) = data.removed.to_owned() {
let form = ModRemoveCommentForm {
mod_user_id: user_id,
@ -360,6 +400,14 @@ impl Perform for Oper<EditComment> {
};
blocking(pool, move |conn| ModRemoveComment::create(conn, &form)).await??;
}
}
}
let post_id = data.post_id;
let post = blocking(pool, move |conn| Post::read(conn, post_id)).await??;
let mentions = scrape_text_for_mentions(&comment_form.content);
let recipient_ids = send_local_notifs(mentions, updated_comment, user, post, pool).await?;
let edit_id = data.edit_id;
let comment_view = blocking(pool, move |conn| {

View file

@ -392,7 +392,7 @@ impl Perform for Oper<EditCommunity> {
title: data.title.to_owned(),
description: data.description.to_owned(),
category_id: data.category_id.to_owned(),
creator_id: user_id,
creator_id: read_community.creator_id,
removed: data.removed.to_owned(),
deleted: data.deleted.to_owned(),
nsfw: data.nsfw,
@ -652,6 +652,28 @@ impl Perform for Oper<BanFromCommunity> {
let user_id = claims.id;
let mut community_moderators: Vec<i32> = vec![];
let community_id = data.community_id;
community_moderators.append(
&mut blocking(pool, move |conn| {
CommunityModeratorView::for_community(&conn, community_id)
.map(|v| v.into_iter().map(|m| m.user_id).collect())
})
.await??,
);
community_moderators.append(
&mut blocking(pool, move |conn| {
UserView::admins(conn).map(|v| v.into_iter().map(|a| a.id).collect())
})
.await??,
);
if !community_moderators.contains(&user_id) {
return Err(APIError::err("couldnt_update_community").into());
}
let community_user_ban_form = CommunityUserBanForm {
community_id: data.community_id,
user_id: data.user_id,
@ -729,6 +751,28 @@ impl Perform for Oper<AddModToCommunity> {
user_id: data.user_id,
};
let mut community_moderators: Vec<i32> = vec![];
let community_id = data.community_id;
community_moderators.append(
&mut blocking(pool, move |conn| {
CommunityModeratorView::for_community(&conn, community_id)
.map(|v| v.into_iter().map(|m| m.user_id).collect())
})
.await??,
);
community_moderators.append(
&mut blocking(pool, move |conn| {
UserView::admins(conn).map(|v| v.into_iter().map(|a| a.id).collect())
})
.await??,
);
if !community_moderators.contains(&user_id) {
return Err(APIError::err("couldnt_update_community").into());
}
if data.added {
let join = move |conn: &'_ _| CommunityModerator::join(conn, &community_moderator_form);
if blocking(pool, join).await?.is_err() {

View file

@ -540,28 +540,36 @@ impl Perform for Oper<EditPost> {
let user_id = claims.id;
let edit_id = data.edit_id;
let read_post = blocking(pool, move |conn| Post::read(conn, edit_id)).await??;
// Verify its the creator or a mod or admin
let community_id = data.community_id;
let mut editors: Vec<i32> = vec![data.creator_id];
editors.append(
let community_id = read_post.community_id;
let mut editors: Vec<i32> = vec![read_post.creator_id];
let mut moderators: Vec<i32> = vec![];
moderators.append(
&mut blocking(pool, move |conn| {
CommunityModeratorView::for_community(conn, community_id)
.map(|v| v.into_iter().map(|m| m.user_id).collect())
})
.await??,
);
editors.append(
moderators.append(
&mut blocking(pool, move |conn| {
UserView::admins(conn).map(|v| v.into_iter().map(|a| a.id).collect())
})
.await??,
);
editors.extend(&moderators);
if !editors.contains(&user_id) {
return Err(APIError::err("no_post_edit_allowed").into());
}
// Check for a community ban
let community_id = data.community_id;
let community_id = read_post.community_id;
let is_banned =
move |conn: &'_ _| CommunityUserBanView::get(conn, user_id, community_id).is_ok();
if blocking(pool, is_banned).await? {
@ -578,15 +586,15 @@ impl Perform for Oper<EditPost> {
let (iframely_title, iframely_description, iframely_html, pictrs_thumbnail) =
fetch_iframely_and_pictrs_data(&self.client, data.url.to_owned()).await;
let edit_id = data.edit_id;
let read_post = blocking(pool, move |conn| Post::read(conn, edit_id)).await??;
let post_form = PostForm {
let post_form = {
// only modify some properties if they are a moderator
if moderators.contains(&user_id) {
PostForm {
name: data.name.trim().to_owned(),
url: data.url.to_owned(),
body: data.body.to_owned(),
creator_id: data.creator_id.to_owned(),
community_id: data.community_id,
creator_id: read_post.creator_id.to_owned(),
community_id: read_post.community_id,
removed: data.removed.to_owned(),
deleted: data.deleted.to_owned(),
nsfw: data.nsfw,
@ -600,6 +608,29 @@ impl Perform for Oper<EditPost> {
ap_id: read_post.ap_id,
local: read_post.local,
published: None,
}
} else {
PostForm {
name: read_post.name.trim().to_owned(),
url: data.url.to_owned(),
body: data.body.to_owned(),
creator_id: read_post.creator_id.to_owned(),
community_id: read_post.community_id,
removed: Some(read_post.removed),
deleted: data.deleted.to_owned(),
nsfw: data.nsfw,
locked: Some(read_post.locked),
stickied: Some(read_post.stickied),
updated: Some(naive_now()),
embed_title: iframely_title,
embed_description: iframely_description,
embed_html: iframely_html,
thumbnail_url: pictrs_thumbnail,
ap_id: read_post.ap_id,
local: read_post.local,
published: None,
}
}
};
let edit_id = data.edit_id;
@ -617,6 +648,7 @@ impl Perform for Oper<EditPost> {
}
};
if moderators.contains(&user_id) {
// Mod tables
if let Some(removed) = data.removed.to_owned() {
let form = ModRemovePostForm {
@ -645,6 +677,7 @@ impl Perform for Oper<EditPost> {
};
blocking(pool, move |conn| ModStickyPost::create(conn, &form)).await??;
}
}
if let Some(deleted) = data.deleted.to_owned() {
if deleted {
@ -655,6 +688,7 @@ impl Perform for Oper<EditPost> {
.await?;
}
} else if let Some(removed) = data.removed.to_owned() {
if moderators.contains(&user_id) {
if removed {
updated_post.send_remove(&user, &self.client, pool).await?;
} else {
@ -662,6 +696,7 @@ impl Perform for Oper<EditPost> {
.send_undo_remove(&user, &self.client, pool)
.await?;
}
}
} else {
updated_post.send_update(&user, &self.client, pool).await?;
}

View file

@ -880,6 +880,9 @@ impl Perform for Oper<EditUserMention> {
};
let user_id = claims.id;
if user_id != data.user_mention_id {
return Err(APIError::err("couldnt_update_comment").into());
}
let user_mention_id = data.user_mention_id;
let user_mention =
@ -1310,23 +1313,35 @@ impl Perform for Oper<EditPrivateMessage> {
let content_slurs_removed = match &data.content {
Some(content) => remove_slurs(content),
None => orig_private_message.content,
None => orig_private_message.content.clone(),
};
let private_message_form = PrivateMessageForm {
let private_message_form = {
if data.read.is_some() {
PrivateMessageForm {
content: orig_private_message.content.to_owned(),
creator_id: orig_private_message.creator_id,
recipient_id: orig_private_message.recipient_id,
read: data.read.to_owned(),
updated: orig_private_message.updated,
deleted: Some(orig_private_message.deleted),
ap_id: orig_private_message.ap_id,
local: orig_private_message.local,
published: None,
}
} else {
PrivateMessageForm {
content: content_slurs_removed,
creator_id: orig_private_message.creator_id,
recipient_id: orig_private_message.recipient_id,
deleted: data.deleted.to_owned(),
read: data.read.to_owned(),
updated: if data.read.is_some() {
orig_private_message.updated
} else {
Some(naive_now())
},
read: Some(orig_private_message.read),
updated: Some(naive_now()),
ap_id: orig_private_message.ap_id,
local: orig_private_message.local,
published: None,
}
}
};
let edit_id = data.edit_id;
@ -1339,6 +1354,7 @@ impl Perform for Oper<EditPrivateMessage> {
Err(_e) => return Err(APIError::err("couldnt_update_private_message").into()),
};
if data.read.is_none() {
if let Some(deleted) = data.deleted.to_owned() {
if deleted {
updated_private_message
@ -1354,6 +1370,11 @@ impl Perform for Oper<EditPrivateMessage> {
.send_update(&user, &self.client, pool)
.await?;
}
} else {
updated_private_message
.send_update(&user, &self.client, pool)
.await?;
}
let edit_id = data.edit_id;
let message = blocking(pool, move |conn| PrivateMessageView::read(conn, edit_id)).await??;

View file

@ -1,16 +1,25 @@
use crate::{
apub::{
activities::{populate_object_props, send_activity_to_community},
create_apub_response, create_apub_tombstone_response, create_tombstone, fetch_webfinger_url,
create_apub_response,
create_apub_tombstone_response,
create_tombstone,
fetch_webfinger_url,
fetcher::{
get_or_fetch_and_insert_remote_comment, get_or_fetch_and_insert_remote_post,
get_or_fetch_and_insert_remote_comment,
get_or_fetch_and_insert_remote_post,
get_or_fetch_and_upsert_remote_user,
},
ActorType, ApubLikeableType, ApubObjectType, FromApub, ToApub,
ActorType,
ApubLikeableType,
ApubObjectType,
FromApub,
ToApub,
},
blocking,
routes::DbPoolParam,
DbPool, LemmyError,
DbPool,
LemmyError,
};
use activitystreams::{
activity::{Create, Delete, Dislike, Like, Remove, Undo, Update},

View file

@ -1,18 +1,28 @@
use crate::{
apub::{
activities::{populate_object_props, send_activity},
create_apub_response, create_apub_tombstone_response, create_tombstone,
create_apub_response,
create_apub_tombstone_response,
create_tombstone,
extensions::group_extensions::GroupExtension,
fetcher::get_or_fetch_and_upsert_remote_user,
get_shared_inbox, insert_activity, ActorType, FromApub, GroupExt, ToApub,
get_shared_inbox,
insert_activity,
ActorType,
FromApub,
GroupExt,
ToApub,
},
blocking,
routes::DbPoolParam,
DbPool, LemmyError,
DbPool,
LemmyError,
};
use activitystreams::{
activity::{Accept, Announce, Delete, Remove, Undo},
Activity, Base, BaseBox,
Activity,
Base,
BaseBox,
};
use activitystreams_ext::Ext2;
use activitystreams_new::{

View file

@ -4,7 +4,8 @@ use crate::{
blocking,
request::{retry, RecvError},
routes::nodeinfo::{NodeInfo, NodeInfoWellKnown},
DbPool, LemmyError,
DbPool,
LemmyError,
};
use activitystreams::object::Note;
use activitystreams_new::{base::BaseExt, prelude::*, primitives::XsdAnyUri};
@ -21,7 +22,9 @@ use lemmy_db::{
post_view::PostView,
user::{UserForm, User_},
user_view::UserView,
Crud, Joinable, SearchType,
Crud,
Joinable,
SearchType,
};
use lemmy_utils::get_apub_protocol_string;
use log::debug;

View file

@ -19,7 +19,8 @@ use crate::{
blocking,
request::{retry, RecvError},
routes::webfinger::WebFingerResponse,
DbPool, LemmyError,
DbPool,
LemmyError,
};
use activitystreams::object::Page;
use activitystreams_ext::{Ext1, Ext2};

View file

@ -1,14 +1,22 @@
use crate::{
apub::{
activities::{populate_object_props, send_activity_to_community},
create_apub_response, create_apub_tombstone_response, create_tombstone,
create_apub_response,
create_apub_tombstone_response,
create_tombstone,
extensions::page_extension::PageExtension,
fetcher::{get_or_fetch_and_upsert_remote_community, get_or_fetch_and_upsert_remote_user},
ActorType, ApubLikeableType, ApubObjectType, FromApub, PageExt, ToApub,
ActorType,
ApubLikeableType,
ApubObjectType,
FromApub,
PageExt,
ToApub,
},
blocking,
routes::DbPoolParam,
DbPool, LemmyError,
DbPool,
LemmyError,
};
use activitystreams::{
activity::{Create, Delete, Dislike, Like, Remove, Undo, Update},

View file

@ -1,9 +1,16 @@
use crate::{
apub::{
activities::send_activity, create_tombstone, fetcher::get_or_fetch_and_upsert_remote_user,
insert_activity, ApubObjectType, FromApub, ToApub,
activities::send_activity,
create_tombstone,
fetcher::get_or_fetch_and_upsert_remote_user,
insert_activity,
ApubObjectType,
FromApub,
ToApub,
},
blocking, DbPool, LemmyError,
blocking,
DbPool,
LemmyError,
};
use activitystreams::{
activity::{Create, Delete, Undo, Update},

View file

@ -8,10 +8,15 @@ use crate::{
community::do_announce,
extensions::signatures::verify,
fetcher::{
get_or_fetch_and_insert_remote_comment, get_or_fetch_and_insert_remote_post,
get_or_fetch_and_upsert_remote_community, get_or_fetch_and_upsert_remote_user,
get_or_fetch_and_insert_remote_comment,
get_or_fetch_and_insert_remote_post,
get_or_fetch_and_upsert_remote_community,
get_or_fetch_and_upsert_remote_user,
},
insert_activity, FromApub, GroupExt, PageExt,
insert_activity,
FromApub,
GroupExt,
PageExt,
},
blocking,
routes::{ChatServerParam, DbPoolParam},
@ -19,12 +24,15 @@ use crate::{
server::{SendComment, SendCommunityRoomMessage, SendPost},
UserOperation,
},
DbPool, LemmyError,
DbPool,
LemmyError,
};
use activitystreams::{
activity::{Announce, Create, Delete, Dislike, Like, Remove, Undo, Update},
object::Note,
Activity, Base, BaseBox,
Activity,
Base,
BaseBox,
};
use actix_web::{client::Client, web, HttpRequest, HttpResponse};
use lemmy_db::{
@ -35,7 +43,8 @@ use lemmy_db::{
naive_now,
post::{Post, PostForm, PostLike, PostLikeForm},
post_view::PostView,
Crud, Likeable,
Crud,
Likeable,
};
use lemmy_utils::scrape_text_for_mentions;
use log::debug;

View file

@ -1,12 +1,18 @@
use crate::{
api::claims::Claims,
apub::{
activities::send_activity, create_apub_response, insert_activity, ActorType, FromApub,
PersonExt, ToApub,
activities::send_activity,
create_apub_response,
insert_activity,
ActorType,
FromApub,
PersonExt,
ToApub,
},
blocking,
routes::DbPoolParam,
DbPool, LemmyError,
DbPool,
LemmyError,
};
use activitystreams_ext::Ext1;
use activitystreams_new::{

View file

@ -3,12 +3,14 @@ use crate::{
apub::{
extensions::signatures::verify,
fetcher::{get_or_fetch_and_upsert_remote_community, get_or_fetch_and_upsert_remote_user},
insert_activity, FromApub,
insert_activity,
FromApub,
},
blocking,
routes::{ChatServerParam, DbPoolParam},
websocket::{server::SendUserRoomMessage, UserOperation},
DbPool, LemmyError,
DbPool,
LemmyError,
};
use activitystreams::{
activity::{Accept, Create, Delete, Undo, Update},
@ -21,7 +23,8 @@ use lemmy_db::{
private_message::{PrivateMessage, PrivateMessageForm},
private_message_view::PrivateMessageView,
user::User_,
Crud, Followable,
Crud,
Followable,
};
use log::debug;
use serde::Deserialize;