mirror of
https://gitlab.computer.surgery/matrix/grapevine.git
synced 2026-02-04 07:41:23 +01:00
validate additional fields for incoming remote membership
This was missed in the initial fix in 9a50c244 ("validate event type and
membership for create_join and create_invite"), but significantly less
impactful than the original vulnerability because it only affects
invite/join events that are able to pass auth/signature checks with our
server's signature. You could use this to forge invite events from a
local user, but not much else.
This commit is contained in:
parent
f29aebbcf4
commit
0aae932bc9
2 changed files with 74 additions and 3 deletions
|
|
@ -63,6 +63,9 @@ This will be the first release of Grapevine since it was forked from Conduit
|
||||||
malicious remote server to trick the local server into signing arbitrary
|
malicious remote server to trick the local server into signing arbitrary
|
||||||
events via remote leave.
|
events via remote leave.
|
||||||
([!206](https://gitlab.computer.surgery/matrix/grapevine/-/merge_requests/206))
|
([!206](https://gitlab.computer.surgery/matrix/grapevine/-/merge_requests/206))
|
||||||
|
10. Fix vulnerability that allows a malicious server to trick a Grapevine server
|
||||||
|
into signing forged invite/join events from local users.
|
||||||
|
([!207](https://gitlab.computer.surgery/matrix/grapevine/-/merge_requests/207))
|
||||||
|
|
||||||
### Removed
|
### Removed
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -59,7 +59,7 @@ use ruma::{
|
||||||
uint, user_id, CanonicalJsonObject, CanonicalJsonValue, EventId,
|
uint, user_id, CanonicalJsonObject, CanonicalJsonValue, EventId,
|
||||||
MilliSecondsSinceUnixEpoch, OwnedEventId, OwnedRoomId, OwnedServerName,
|
MilliSecondsSinceUnixEpoch, OwnedEventId, OwnedRoomId, OwnedServerName,
|
||||||
OwnedServerSigningKeyId, OwnedSigningKeyId, OwnedUserId, RoomId,
|
OwnedServerSigningKeyId, OwnedSigningKeyId, OwnedUserId, RoomId,
|
||||||
ServerName, Signatures,
|
ServerName, Signatures, UserId,
|
||||||
};
|
};
|
||||||
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;
|
||||||
|
|
@ -1651,7 +1651,12 @@ async fn create_join_event(
|
||||||
));
|
));
|
||||||
};
|
};
|
||||||
|
|
||||||
validate_remote_member_event(&MembershipState::Join, &value)?;
|
validate_remote_member_event(
|
||||||
|
&MembershipState::Join,
|
||||||
|
sender_servername,
|
||||||
|
room_id,
|
||||||
|
&value,
|
||||||
|
)?;
|
||||||
|
|
||||||
let origin: OwnedServerName = serde_json::from_value(
|
let origin: OwnedServerName = serde_json::from_value(
|
||||||
serde_json::to_value(value.get("origin").ok_or(Error::BadRequest(
|
serde_json::to_value(value.get("origin").ok_or(Error::BadRequest(
|
||||||
|
|
@ -1776,6 +1781,8 @@ pub(crate) async fn create_join_event_v2_route(
|
||||||
/// endpoints to trick our server into signing arbitrary malicious events.
|
/// endpoints to trick our server into signing arbitrary malicious events.
|
||||||
fn validate_remote_member_event(
|
fn validate_remote_member_event(
|
||||||
membership: &MembershipState,
|
membership: &MembershipState,
|
||||||
|
origin: &ServerName,
|
||||||
|
room_id: &RoomId,
|
||||||
event: &CanonicalJsonObject,
|
event: &CanonicalJsonObject,
|
||||||
) -> Result<()> {
|
) -> Result<()> {
|
||||||
let Some(event_type) = event.get("type") else {
|
let Some(event_type) = event.get("type") else {
|
||||||
|
|
@ -1791,6 +1798,62 @@ fn validate_remote_member_event(
|
||||||
));
|
));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
let Some(sender) = event.get("sender").and_then(|sender| sender.as_str())
|
||||||
|
else {
|
||||||
|
return Err(Error::BadRequest(
|
||||||
|
ErrorKind::InvalidParam,
|
||||||
|
"Event sender property is missing or not a string",
|
||||||
|
));
|
||||||
|
};
|
||||||
|
let Ok(sender) = UserId::parse(sender) else {
|
||||||
|
return Err(Error::BadRequest(
|
||||||
|
ErrorKind::InvalidParam,
|
||||||
|
"Event sender is not a valid user ID",
|
||||||
|
));
|
||||||
|
};
|
||||||
|
if sender.server_name() != origin {
|
||||||
|
return Err(Error::BadRequest(
|
||||||
|
ErrorKind::InvalidParam,
|
||||||
|
"Event sender is not on the server that sent the request",
|
||||||
|
));
|
||||||
|
}
|
||||||
|
|
||||||
|
if membership == &MembershipState::Invite {
|
||||||
|
let Some(state_key) =
|
||||||
|
event.get("state_key").and_then(|state_key| state_key.as_str())
|
||||||
|
else {
|
||||||
|
return Err(Error::BadRequest(
|
||||||
|
ErrorKind::InvalidParam,
|
||||||
|
"Event state_key property is missing or not a string",
|
||||||
|
));
|
||||||
|
};
|
||||||
|
let Ok(state_key) = UserId::parse(state_key) else {
|
||||||
|
return Err(Error::BadRequest(
|
||||||
|
ErrorKind::InvalidParam,
|
||||||
|
"Event state_key is not a valid user ID",
|
||||||
|
));
|
||||||
|
};
|
||||||
|
if state_key.server_name() != services().globals.server_name() {
|
||||||
|
return Err(Error::BadRequest(
|
||||||
|
ErrorKind::InvalidParam,
|
||||||
|
"Invite event recipient is not a user on this server",
|
||||||
|
));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
let Some(event_room_id) = event.get("room_id") else {
|
||||||
|
return Err(Error::BadRequest(
|
||||||
|
ErrorKind::InvalidParam,
|
||||||
|
"Event missing room_id property",
|
||||||
|
));
|
||||||
|
};
|
||||||
|
if event_room_id != &room_id.as_str() {
|
||||||
|
return Err(Error::BadRequest(
|
||||||
|
ErrorKind::InvalidParam,
|
||||||
|
"Event room_id does not match room ID in endpoint url",
|
||||||
|
));
|
||||||
|
}
|
||||||
|
|
||||||
let Some(content) =
|
let Some(content) =
|
||||||
event.get("content").and_then(|content| content.as_object())
|
event.get("content").and_then(|content| content.as_object())
|
||||||
else {
|
else {
|
||||||
|
|
@ -1856,7 +1919,12 @@ pub(crate) async fn create_invite_route(
|
||||||
)
|
)
|
||||||
})?;
|
})?;
|
||||||
|
|
||||||
validate_remote_member_event(&MembershipState::Invite, &signed_event)?;
|
validate_remote_member_event(
|
||||||
|
&MembershipState::Invite,
|
||||||
|
sender_servername,
|
||||||
|
&body.room_id,
|
||||||
|
&signed_event,
|
||||||
|
)?;
|
||||||
|
|
||||||
ruma::signatures::hash_and_sign_event(
|
ruma::signatures::hash_and_sign_event(
|
||||||
services().globals.server_name().as_str(),
|
services().globals.server_name().as_str(),
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue