mirror of
https://gitlab.computer.surgery/matrix/grapevine.git
synced 2025-12-22 18:21:24 +01:00
validate event type and membership for create_join and create_invite
Both of these endpoints sign the received event so without the validation a malicious server can use these endpoints to trick our server into signing effectively arbitrary forged events from local users. Rebased from a continuwuity patch by nex. The create_join changes were not present in the continuwuity version because these checks were already present there. Co-authored-by: Olivia Lee <olivia@computer.surgery>
This commit is contained in:
parent
c4abca1eb5
commit
9a50c2448a
2 changed files with 74 additions and 1 deletions
|
|
@ -56,6 +56,9 @@ This will be the first release of Grapevine since it was forked from Conduit
|
||||||
7. Only allow the admin bot to change the room ID that the admin room alias
|
7. Only allow the admin bot to change the room ID that the admin room alias
|
||||||
points to.
|
points to.
|
||||||
([!42](https://gitlab.computer.surgery/matrix/grapevine/-/merge_requests/42))
|
([!42](https://gitlab.computer.surgery/matrix/grapevine/-/merge_requests/42))
|
||||||
|
8. Fix vulnerability that allows a malicious server to trick a grapevine server
|
||||||
|
into signing arbitrary forged events via the send_invite endpoint.
|
||||||
|
([!205](https://gitlab.computer.surgery/matrix/grapevine/-/merge_requests/205))
|
||||||
|
|
||||||
### Removed
|
### Removed
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -51,7 +51,7 @@ use ruma::{
|
||||||
join_rules::{JoinRule, RoomJoinRulesEventContent},
|
join_rules::{JoinRule, RoomJoinRulesEventContent},
|
||||||
member::{MembershipState, RoomMemberEventContent},
|
member::{MembershipState, RoomMemberEventContent},
|
||||||
},
|
},
|
||||||
StateEventType, TimelineEventType,
|
StateEventType, StaticEventContent, TimelineEventType,
|
||||||
},
|
},
|
||||||
serde::{Base64, JsonObject, Raw},
|
serde::{Base64, JsonObject, Raw},
|
||||||
state_res::Event,
|
state_res::Event,
|
||||||
|
|
@ -61,6 +61,7 @@ use ruma::{
|
||||||
OwnedServerSigningKeyId, OwnedSigningKeyId, OwnedUserId, RoomId,
|
OwnedServerSigningKeyId, OwnedSigningKeyId, OwnedUserId, RoomId,
|
||||||
ServerName, Signatures,
|
ServerName, Signatures,
|
||||||
};
|
};
|
||||||
|
use serde::Deserialize;
|
||||||
use serde_json::value::{to_raw_value, RawValue as RawJsonValue};
|
use serde_json::value::{to_raw_value, RawValue as RawJsonValue};
|
||||||
use tokio::sync::RwLock;
|
use tokio::sync::RwLock;
|
||||||
use tracing::{debug, error, field, trace, trace_span, warn};
|
use tracing::{debug, error, field, trace, trace_span, warn};
|
||||||
|
|
@ -1582,6 +1583,13 @@ async fn create_join_event(
|
||||||
room_id: &RoomId,
|
room_id: &RoomId,
|
||||||
pdu: &RawJsonValue,
|
pdu: &RawJsonValue,
|
||||||
) -> Result<create_join_event::v2::RoomState> {
|
) -> Result<create_join_event::v2::RoomState> {
|
||||||
|
#[derive(Deserialize)]
|
||||||
|
struct ExtractPdu<'a> {
|
||||||
|
#[serde(rename = "type")]
|
||||||
|
event_type: &'a str,
|
||||||
|
content: RoomMemberEventContent,
|
||||||
|
}
|
||||||
|
|
||||||
if !services().rooms.metadata.exists(room_id)? {
|
if !services().rooms.metadata.exists(room_id)? {
|
||||||
return Err(Error::BadRequest(
|
return Err(Error::BadRequest(
|
||||||
ErrorKind::NotFound,
|
ErrorKind::NotFound,
|
||||||
|
|
@ -1591,6 +1599,28 @@ async fn create_join_event(
|
||||||
|
|
||||||
services().rooms.event_handler.acl_check(sender_servername, room_id)?;
|
services().rooms.event_handler.acl_check(sender_servername, room_id)?;
|
||||||
|
|
||||||
|
let extract: ExtractPdu<'_> =
|
||||||
|
serde_json::from_str(pdu.get()).map_err(|_| {
|
||||||
|
Error::BadRequest(
|
||||||
|
ErrorKind::InvalidParam,
|
||||||
|
"Event does not match expected schema",
|
||||||
|
)
|
||||||
|
})?;
|
||||||
|
|
||||||
|
if extract.event_type != RoomMemberEventContent::TYPE {
|
||||||
|
return Err(Error::BadRequest(
|
||||||
|
ErrorKind::InvalidParam,
|
||||||
|
"Event is not a membership event",
|
||||||
|
));
|
||||||
|
}
|
||||||
|
if extract.content.membership != MembershipState::Join {
|
||||||
|
return Err(Error::BadRequest(
|
||||||
|
ErrorKind::InvalidParam,
|
||||||
|
"Not allowed to send a non-join membership event to send_join \
|
||||||
|
endpoint.",
|
||||||
|
));
|
||||||
|
}
|
||||||
|
|
||||||
// TODO: Grapevine does not implement restricted join rules yet, we always
|
// TODO: Grapevine does not implement restricted join rules yet, we always
|
||||||
// reject
|
// reject
|
||||||
let join_rules_event = services().rooms.state_accessor.room_state_get(
|
let join_rules_event = services().rooms.state_accessor.room_state_get(
|
||||||
|
|
@ -1807,6 +1837,46 @@ pub(crate) async fn create_invite_route(
|
||||||
)
|
)
|
||||||
})?;
|
})?;
|
||||||
|
|
||||||
|
let event_type = signed_event.get("type").ok_or_else(|| {
|
||||||
|
Error::BadRequest(
|
||||||
|
ErrorKind::InvalidParam,
|
||||||
|
"Event missing type property",
|
||||||
|
)
|
||||||
|
})?;
|
||||||
|
if event_type.as_str() != Some(RoomMemberEventContent::TYPE) {
|
||||||
|
return Err(Error::BadRequest(
|
||||||
|
ErrorKind::InvalidParam,
|
||||||
|
"Event is not a membership event",
|
||||||
|
));
|
||||||
|
}
|
||||||
|
|
||||||
|
let content: RoomMemberEventContent = serde_json::from_value(
|
||||||
|
signed_event
|
||||||
|
.get("content")
|
||||||
|
.ok_or_else(|| {
|
||||||
|
Error::BadRequest(
|
||||||
|
ErrorKind::InvalidParam,
|
||||||
|
"Event missing content property",
|
||||||
|
)
|
||||||
|
})?
|
||||||
|
.clone()
|
||||||
|
.into(),
|
||||||
|
)
|
||||||
|
.map_err(|_| {
|
||||||
|
Error::BadRequest(
|
||||||
|
ErrorKind::InvalidParam,
|
||||||
|
"Event content is empty or invalid",
|
||||||
|
)
|
||||||
|
})?;
|
||||||
|
|
||||||
|
if content.membership != MembershipState::Invite {
|
||||||
|
return Err(Error::BadRequest(
|
||||||
|
ErrorKind::InvalidParam,
|
||||||
|
"Not allowed to send a non-invite membership event to invite \
|
||||||
|
endpoint.",
|
||||||
|
));
|
||||||
|
}
|
||||||
|
|
||||||
ruma::signatures::hash_and_sign_event(
|
ruma::signatures::hash_and_sign_event(
|
||||||
services().globals.server_name().as_str(),
|
services().globals.server_name().as_str(),
|
||||||
services().globals.keypair(),
|
services().globals.keypair(),
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue