From 103a4fb56b4fd3fd852076e72de96d87304dc14e Mon Sep 17 00:00:00 2001 From: Olivia Lee Date: Wed, 27 Nov 2024 22:10:06 -0800 Subject: [PATCH] handle media keys where thumbnail size contains 0xFF Our current code should never write these, because we have an allowlist of thumbnail sizes. None of the allowed sizes contain a 0xFF byte. We have observed keys with a 0xFF in the thumbnail size a couple times on real servers, and believe an early version of conduit wrote these before the allowlist was added. These keys were originally handled correctly, and were broken by e2cba15ed2ec1681cc455d66a7bb2053cc204f23. Before that commit, we were parsing media keys backwards, and never tried to read the thumbnail size or mxc url. --- src/database/key_value/media.rs | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/src/database/key_value/media.rs b/src/database/key_value/media.rs index 9fc15e54..713c8b05 100644 --- a/src/database/key_value/media.rs +++ b/src/database/key_value/media.rs @@ -24,18 +24,37 @@ impl TryFrom<&MediaFileKey> for MediaFileKeyParts { fields(key = utils::u8_slice_to_hex(key.as_bytes())), )] fn try_from(key: &MediaFileKey) -> Result { - let mut parts = key.as_bytes().split(|&b| b == 0xFF); - // Extract parts + // Extracting mxc url and thumbnail size is a bit fiddly, because the + // thumbnail size may contain 0xFF bytes. + let mut parts = key.as_bytes().splitn(2, |&b| b == 0xFF); + let mxc_bytes = parts .next() .ok_or_else(|| Error::BadDatabase("Missing MXC URI bytes"))?; - let thumbnail_size_bytes = parts.next().ok_or_else(|| { + let rest = parts.next().ok_or_else(|| { Error::BadDatabase("Missing thumbnail size bytes") })?; + // Thumbnail size is always 8 bytes + let (thumbnail_size_bytes, rest) = + rest.split_at_checked(8).ok_or_else(|| { + Error::BadDatabase("Missing thumbnail size bytes") + })?; + + // And always followed immediately by a 0xFF separator + let mut parts = rest.split(|&b| b == 0xFF); + + let thumbnail_size_rest = parts.next().ok_or_else(|| { + Error::BadDatabase("Missing Content-Disposition bytes") + })?; + if !thumbnail_size_rest.is_empty() { + return Err(Error::BadDatabase("Thumbnail size part is too long")); + } + + // The remaining parts are straightforward 0xFF-delimited fields let content_disposition_bytes = parts.next().ok_or_else(|| { Error::BadDatabase("Missing Content-Disposition bytes") })?;