base: Fix deserialization failure leading to database corruption

f36a5b8cd7 introduced
deserialize_events, moving the deserialization out of handle_state.
However, the new code in deserialize_events used filter_map, which
caused a deserialization failure to lead to the indices of the
serialized and deserialized events to no longer match up.

This was not caught because iter::zip also does not require that its
arguments have matching lengths, causing the mismatch to cascade for
that batch of events, and persist in the store.  An application
affected by this form of corruption can, for example, call
room.get_state_events requesting events of a certain type, and getting
back events of a different type.

Fix the bug by using Option to preserve the length of
deserialize_events' return value, and add an assertion to ensure
handle_state's contract.

Signed-off-by: Vladimir Panteleev <git@cy.md>
This commit is contained in:
Vladimir Panteleev
2023-07-08 15:06:25 +00:00
committed by Benjamin Bouvier
parent 6ea806bf18
commit ab781bf5f4
2 changed files with 11 additions and 6 deletions

View File

@@ -453,7 +453,7 @@ impl BaseClient {
pub(crate) async fn handle_state(
&self,
raw_events: &[Raw<AnySyncStateEvent>],
events: &[AnySyncStateEvent],
events: &[Option<AnySyncStateEvent>],
room_info: &mut RoomInfo,
changes: &mut StateChanges,
ambiguity_cache: &mut AmbiguityCache,
@@ -462,7 +462,12 @@ impl BaseClient {
let mut user_ids = BTreeSet::new();
let mut profiles = BTreeMap::new();
assert_eq!(raw_events.len(), events.len());
for (raw_event, event) in iter::zip(raw_events, events) {
let Some(event) = event else {
continue;
};
room_info.handle_state_event(event);
if let AnySyncStateEvent::RoomMember(member) = &event {
@@ -1268,10 +1273,10 @@ impl BaseClient {
pub(crate) fn deserialize_events(
raw_events: &[Raw<AnySyncStateEvent>],
) -> Vec<AnySyncStateEvent> {
) -> Vec<Option<AnySyncStateEvent>> {
raw_events
.iter()
.filter_map(|raw_event| match raw_event.deserialize() {
.map(|raw_event| match raw_event.deserialize() {
Ok(ev) => Some(ev),
Err(e) => {
warn!("Couldn't deserialize state event: {e}");

View File

@@ -299,7 +299,7 @@ impl BaseClient {
fn process_sliding_sync_room_membership(
&self,
room_data: &v4::SlidingSyncRoom,
required_state: &[AnySyncStateEvent],
required_state: &[Option<AnySyncStateEvent>],
store: &Store,
room_id: &RoomId,
changes: &mut StateChanges,
@@ -353,11 +353,11 @@ impl BaseClient {
/// the state in room_info to reflect the "membership" property.
pub(crate) fn handle_own_room_membership(
&self,
required_state: &[AnySyncStateEvent],
required_state: &[Option<AnySyncStateEvent>],
room_info: &mut RoomInfo,
) {
for event in required_state {
if let AnySyncStateEvent::RoomMember(member) = &event {
if let Some(AnySyncStateEvent::RoomMember(member)) = &event {
// If this event updates the current user's membership, record that in the
// room_info.
if let Some(meta) = self.session_meta() {