fix(sqlite): Fix a SQL injection issue in the find_event_relations function

poljar · poljar · commit d0c01006e480 · 2025-07-10T13:55:02.000+02:00
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
diff --git a/crates/matrix-sdk-sqlite/src/event_cache_store.rs b/crates/matrix-sdk-sqlite/src/event_cache_store.rs
@@ -1156,52 +1156,75 @@ impl EventCacheStore for SqliteEventCacheStore {
         self.acquire()
             .await?
             .with_transaction(move |txn| -> Result<_> {
-                let filter_query = if let Some(filters) = compute_filters_string(filters.as_deref())
-                {
-                    format!(
-                        " AND rel_type IN ({})",
-                        filters
-                            .into_iter()
-                            .map(|f| format!(r#""{f}""#))
-                            .collect::<Vec<_>>()
-                            .join(", ")
-                    )
-                } else {
-                    "".to_owned()
-                };
-
-                let query = format!(
-                    "SELECT events.content, event_chunks.chunk_id, event_chunks.position
-                    FROM events
-                    LEFT JOIN event_chunks ON events.event_id = event_chunks.event_id AND event_chunks.linked_chunk_id = ?
-                    WHERE relates_to = ? AND room_id = ? {filter_query}"
-                );
-
-                // Collect related events.
-                let mut related = Vec::new();
-                for result in
-                    txn.prepare(&query)?.query_map((hashed_linked_chunk_id, event_id.as_str(), hashed_room_id), |row| {
-                        Ok((
+                let get_rows = |row: &rusqlite::Row<'_>| {
+                    Ok((
                             row.get::<_, Vec<u8>>(0)?,
                             row.get::<_, Option<u64>>(1)?,
                             row.get::<_, Option<usize>>(2)?,
-                        ))
-                    })?
-                {
-                    let (event_blob, chunk_id, index) = result?;
+                    ))
+                };
 
-                    let event: Event = serde_json::from_slice(&this.decode_value(&event_blob)?)?;
+                // Collect related events.
+                let collect_results = |transaction| {
+                    let mut related = Vec::new();
 
-                    // Only build the position if both the chunk_id and position were present; in
-                    // theory, they should either be present at the same time, or not at all.
-                    let pos = chunk_id.zip(index).map(|(chunk_id, index)| {
-                        Position::new(ChunkIdentifier::new(chunk_id), index)
-                    });
+                    for result in transaction {
+                        let (event_blob, chunk_id, index): (Vec<u8>, Option<u64>, _) = result?;
 
-                    related.push((event, pos));
-                }
+                        let event: Event = serde_json::from_slice(&this.decode_value(&event_blob)?)?;
+
+                        // Only build the position if both the chunk_id and position were present; in
+                        // theory, they should either be present at the same time, or not at all.
+                        let pos = chunk_id.zip(index).map(|(chunk_id, index)| {
+                            Position::new(ChunkIdentifier::new(chunk_id), index)
+                        });
+
+                        related.push((event, pos));
+                    }
+
+                    Ok(related)
+                };
+
+                let related = if let Some(filters) = compute_filters_string(filters.as_deref()) {
+                    let question_marks = repeat_vars(filters.len());
+                    let query = format!(
+                        "SELECT events.content, event_chunks.chunk_id, event_chunks.position
+                        FROM events
+                        LEFT JOIN event_chunks ON events.event_id = event_chunks.event_id AND event_chunks.linked_chunk_id = ?
+                        WHERE relates_to = ? AND room_id = ? AND rel_type IN ({question_marks})"
+                    );
+
+                    let filters: Vec<_> = filters.iter().map(|f| f.to_sql().unwrap()).collect();
+                    let parameters = params_from_iter(
+                        [
+                            hashed_linked_chunk_id.to_sql().expect("We should be able to convert a hashed linked chunk ID to a SQLite value"),
+                            event_id.as_str().to_sql().expect("We should be able to convert an event ID to a SQLite value"),
+                            hashed_room_id.to_sql().expect("We should be able to convert a room ID to a SQLite value")
+                        ]
+                            .into_iter()
+                            .chain(filters)
+                    );
+
+                    let mut transaction = txn.prepare(&query)?;
+                    let transaction = transaction.query_map(parameters, get_rows)?;
+
+                    collect_results(transaction)
+
+                } else {
+                    let query =
+                        "SELECT events.content, event_chunks.chunk_id, event_chunks.position
+                        FROM events
+                        LEFT JOIN event_chunks ON events.event_id = event_chunks.event_id AND event_chunks.linked_chunk_id = ?
+                        WHERE relates_to = ? AND room_id = ?";
+                    let parameters = (hashed_linked_chunk_id, event_id.as_str(), hashed_room_id);
+
+                    let mut transaction = txn.prepare(query)?;
+                    let transaction = transaction.query_map(parameters, get_rows)?;
+
+                    collect_results(transaction)
+                };
 
-                Ok(related)
+                related
             })
             .await
     }
