Skip to content

Commit d0c0100

Browse files
committed
fix(sqlite): Fix a SQL injection issue in the find_event_relations function
The SQLite implementation for the EventCache::find_event_with_relations() the relation type list isn't inserted using SQL placeholders. The relation types are inserted manually using a format!() call. The usage of the format!() call can lead to SQL injection if a RelationType::Custom variant is used which contains SQL expressions. This patch modifies the, query logic which retrieves the related events, to use two separate queries which use SQL placeholders to insert all the dynamic variables. Security-Impact: Moderate CVE: CVE-2025-53549 GitHub-Advisory: GHSA-275g-g844-73jh
1 parent dc98bf7 commit d0c0100

File tree

1 file changed

+62
-39
lines changed

1 file changed

+62
-39
lines changed

crates/matrix-sdk-sqlite/src/event_cache_store.rs

Lines changed: 62 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1156,52 +1156,75 @@ impl EventCacheStore for SqliteEventCacheStore {
11561156
self.acquire()
11571157
.await?
11581158
.with_transaction(move |txn| -> Result<_> {
1159-
let filter_query = if let Some(filters) = compute_filters_string(filters.as_deref())
1160-
{
1161-
format!(
1162-
" AND rel_type IN ({})",
1163-
filters
1164-
.into_iter()
1165-
.map(|f| format!(r#""{f}""#))
1166-
.collect::<Vec<_>>()
1167-
.join(", ")
1168-
)
1169-
} else {
1170-
"".to_owned()
1171-
};
1172-
1173-
let query = format!(
1174-
"SELECT events.content, event_chunks.chunk_id, event_chunks.position
1175-
FROM events
1176-
LEFT JOIN event_chunks ON events.event_id = event_chunks.event_id AND event_chunks.linked_chunk_id = ?
1177-
WHERE relates_to = ? AND room_id = ? {filter_query}"
1178-
);
1179-
1180-
// Collect related events.
1181-
let mut related = Vec::new();
1182-
for result in
1183-
txn.prepare(&query)?.query_map((hashed_linked_chunk_id, event_id.as_str(), hashed_room_id), |row| {
1184-
Ok((
1159+
let get_rows = |row: &rusqlite::Row<'_>| {
1160+
Ok((
11851161
row.get::<_, Vec<u8>>(0)?,
11861162
row.get::<_, Option<u64>>(1)?,
11871163
row.get::<_, Option<usize>>(2)?,
1188-
))
1189-
})?
1190-
{
1191-
let (event_blob, chunk_id, index) = result?;
1164+
))
1165+
};
11921166

1193-
let event: Event = serde_json::from_slice(&this.decode_value(&event_blob)?)?;
1167+
// Collect related events.
1168+
let collect_results = |transaction| {
1169+
let mut related = Vec::new();
11941170

1195-
// Only build the position if both the chunk_id and position were present; in
1196-
// theory, they should either be present at the same time, or not at all.
1197-
let pos = chunk_id.zip(index).map(|(chunk_id, index)| {
1198-
Position::new(ChunkIdentifier::new(chunk_id), index)
1199-
});
1171+
for result in transaction {
1172+
let (event_blob, chunk_id, index): (Vec<u8>, Option<u64>, _) = result?;
12001173

1201-
related.push((event, pos));
1202-
}
1174+
let event: Event = serde_json::from_slice(&this.decode_value(&event_blob)?)?;
1175+
1176+
// Only build the position if both the chunk_id and position were present; in
1177+
// theory, they should either be present at the same time, or not at all.
1178+
let pos = chunk_id.zip(index).map(|(chunk_id, index)| {
1179+
Position::new(ChunkIdentifier::new(chunk_id), index)
1180+
});
1181+
1182+
related.push((event, pos));
1183+
}
1184+
1185+
Ok(related)
1186+
};
1187+
1188+
let related = if let Some(filters) = compute_filters_string(filters.as_deref()) {
1189+
let question_marks = repeat_vars(filters.len());
1190+
let query = format!(
1191+
"SELECT events.content, event_chunks.chunk_id, event_chunks.position
1192+
FROM events
1193+
LEFT JOIN event_chunks ON events.event_id = event_chunks.event_id AND event_chunks.linked_chunk_id = ?
1194+
WHERE relates_to = ? AND room_id = ? AND rel_type IN ({question_marks})"
1195+
);
1196+
1197+
let filters: Vec<_> = filters.iter().map(|f| f.to_sql().unwrap()).collect();
1198+
let parameters = params_from_iter(
1199+
[
1200+
hashed_linked_chunk_id.to_sql().expect("We should be able to convert a hashed linked chunk ID to a SQLite value"),
1201+
event_id.as_str().to_sql().expect("We should be able to convert an event ID to a SQLite value"),
1202+
hashed_room_id.to_sql().expect("We should be able to convert a room ID to a SQLite value")
1203+
]
1204+
.into_iter()
1205+
.chain(filters)
1206+
);
1207+
1208+
let mut transaction = txn.prepare(&query)?;
1209+
let transaction = transaction.query_map(parameters, get_rows)?;
1210+
1211+
collect_results(transaction)
1212+
1213+
} else {
1214+
let query =
1215+
"SELECT events.content, event_chunks.chunk_id, event_chunks.position
1216+
FROM events
1217+
LEFT JOIN event_chunks ON events.event_id = event_chunks.event_id AND event_chunks.linked_chunk_id = ?
1218+
WHERE relates_to = ? AND room_id = ?";
1219+
let parameters = (hashed_linked_chunk_id, event_id.as_str(), hashed_room_id);
1220+
1221+
let mut transaction = txn.prepare(query)?;
1222+
let transaction = transaction.query_map(parameters, get_rows)?;
1223+
1224+
collect_results(transaction)
1225+
};
12031226

1204-
Ok(related)
1227+
related
12051228
})
12061229
.await
12071230
}

0 commit comments

Comments
 (0)