Remove exif info from image uploads (#352)

* remove exif info during file uploads

* add some space

* formatting issue

* add a comment about mime types

---------

Co-authored-by: aumetra <[email protected]>

Author: zeerooth

crates/kitsune-core/Cargo.toml

@@ -34,6 +34,7 @@ garde = { version = "0.15.0", default-features = false, features = [
 globset = "0.4.13"
 hex-simd = "0.8.0"
 http = "0.2.9"
+img-parts = "0.3.0"
 iso8601-timestamp = "0.2.12"
 just-retry = { path = "../../lib/just-retry" }
 kitsune-cache = { path = "../kitsune-cache" }
@@ -90,4 +91,5 @@ kitsune-test = { path = "../kitsune-test" }
 pretty_assertions = "1.4.0"
 redis = "0.23.3"
 serial_test = "2.0.0"
+tempfile = "3.8.0"
 tower = "0.4.13"

crates/kitsune-core/src/error.rs

@@ -1,7 +1,10 @@
+use std::error::Error as ErrorTrait;
+
 use kitsune_http_signatures::ring;
 use thiserror::Error;
 use tokio::sync::oneshot;
 
+pub type BoxError = Box<dyn ErrorTrait + Send + Sync>;
 pub type Result<T, E = Error> = std::result::Result<T, E>;
 
 #[derive(Debug, Error)]
@@ -46,6 +49,15 @@ pub enum FederationFilterError {
     UrlParse(#[from] url::ParseError),
 }
 
+#[derive(Debug, Error)]
+pub enum UploadError {
+    #[error(transparent)]
+    ImageProcessingError(#[from] img_parts::Error),
+
+    #[error(transparent)]
+    StreamError(#[from] BoxError),
+}
+
 #[derive(Debug, Error)]
 pub enum Error {
     #[error(transparent)]
@@ -120,6 +132,9 @@ pub enum Error {
     #[error(transparent)]
     TokioOneshot(#[from] oneshot::error::RecvError),
 
+    #[error(transparent)]
+    Upload(#[from] UploadError),
+
     #[error(transparent)]
     UriInvalid(#[from] http::uri::InvalidUri),
 

crates/kitsune-core/src/service/attachment.rs

@@ -1,14 +1,15 @@
 use super::url::UrlService;
 use crate::{
     consts::{MAX_MEDIA_DESCRIPTION_LENGTH, USER_AGENT},
-    error::{ApiError, Error, Result},
+    error::{ApiError, Error, Result, UploadError},
 };
-use bytes::Bytes;
+use bytes::{Bytes, BytesMut};
 use derive_builder::Builder;
 use diesel::{BoolExpressionMethods, ExpressionMethods, QueryDsl};
 use diesel_async::RunQueryDsl;
-use futures_util::{Stream, StreamExt, TryStreamExt};
+use futures_util::{pin_mut, stream, Stream, StreamExt, TryStreamExt};
 use garde::Validate;
+use img_parts::{DynImage, ImageEXIF};
 use kitsune_db::{
     model::media_attachment::{MediaAttachment, NewMediaAttachment, UpdateMediaAttachment},
     schema::media_attachments,
@@ -35,6 +36,11 @@ fn is_allowed_filetype(value: &str, _ctx: &()) -> garde::Result {
     Ok(())
 }
 
+fn is_image_type_with_supported_metadata(mime: &str) -> bool {
+    // TODO: migrate the match to use the mime crate enums
+    matches!(mime, "image/jpeg" | "image/png" | "image/webp")
+}
+
 #[derive(TypedBuilder, Validate)]
 pub struct Update {
     #[garde(skip)]
@@ -179,10 +185,38 @@ impl AttachmentService {
     {
         upload.validate(&())?;
 
-        self.storage_backend
-            .put(&upload.path, upload.stream)
-            .await
-            .map_err(Error::Storage)?;
+        // remove exif info from image uploads
+        let upload_stream = if is_image_type_with_supported_metadata(&upload.content_type) {
+            let stream = upload.stream;
+            pin_mut!(stream);
+
+            let mut img_bytes = BytesMut::new();
+            while let Some(chunk) = stream
+                .next()
+                .await
+                .transpose()
+                .map_err(UploadError::StreamError)?
+            {
+                img_bytes.extend_from_slice(&chunk);
+            }
+
+            let img_bytes = img_bytes.freeze();
+            let final_bytes = DynImage::from_bytes(img_bytes)
+                .map_err(UploadError::ImageProcessingError)?
+                .ok_or(img_parts::Error::WrongSignature)
+                .map(|mut image| {
+                    image.set_exif(None);
+                    image.encoder().bytes()
+                })
+                .map_err(UploadError::ImageProcessingError)?;
+
+            self.storage_backend
+                .put(&upload.path, stream::once(async { Ok(final_bytes) }))
+        } else {
+            self.storage_backend.put(&upload.path, upload.stream)
+        };
+
+        upload_stream.await.map_err(Error::Storage)?;
 
         let media_attachment = self
             .db_pool
@@ -205,3 +239,153 @@ impl AttachmentService {
         Ok(media_attachment)
     }
 }
+
+#[cfg(test)]
+mod test {
+    use std::convert::Infallible;
+
+    use bytes::{Bytes, BytesMut};
+    use diesel_async::{AsyncConnection, AsyncPgConnection, RunQueryDsl};
+    use futures_util::{future, pin_mut, stream, StreamExt};
+    use http::{Request, Response};
+    use hyper::Body;
+    use img_parts::{
+        jpeg::{markers, JpegSegment},
+        ImageEXIF,
+    };
+    use iso8601_timestamp::Timestamp;
+    use kitsune_db::{
+        model::{
+            account::{ActorType, NewAccount},
+            media_attachment::MediaAttachment,
+        },
+        schema::accounts,
+    };
+    use kitsune_http_client::Client;
+    use kitsune_storage::fs::Storage;
+    use kitsune_test::database_test;
+    use scoped_futures::ScopedFutureExt;
+    use speedy_uuid::Uuid;
+    use tempfile::TempDir;
+    use tower::service_fn;
+
+    use crate::{
+        error::Error,
+        service::{
+            attachment::{AttachmentService, Upload},
+            url::UrlService,
+        },
+    };
+
+    #[tokio::test]
+    #[serial_test::serial]
+    async fn upload_jpeg() {
+        database_test(|db_pool| async move {
+            let client = Client::builder().service(service_fn(handle));
+
+            let account_id = db_pool
+                .with_connection(|db_conn| {
+                    async move { Ok::<_, eyre::Report>(prepare_db(db_conn).await) }.scoped()
+                })
+                .await
+                .unwrap();
+
+            let temp_dir = TempDir::new().unwrap();
+            let storage = Storage::new(temp_dir.path().to_owned());
+            let url_service = UrlService::builder()
+                .domain("example.com")
+                .scheme("http")
+                .build();
+
+            let attachment_service = AttachmentService::builder()
+                .client(client)
+                .db_pool(db_pool)
+                .url_service(url_service)
+                .storage_backend(storage)
+                .media_proxy_enabled(false)
+                .build();
+
+            let base = hex_simd::decode_to_vec("ffd8ffe000104a46494600010101004800480000ffdb004300030202020202030202020303030304060404040404080606050609080a0a090809090a0c0f0c0a0b0e0b09090d110d0e0f101011100a0c12131210130f101010ffc9000b080001000101011100ffcc000600101005ffda0008010100003f00d2cf20ffd9").unwrap();
+            let mut jpeg = img_parts::jpeg::Jpeg::from_bytes(Bytes::from(base)).unwrap();
+
+            let comment_segment = JpegSegment::new_with_contents(
+                markers::APP1,
+                Bytes::from("Exif\0\0Some info to be stripped")
+            );
+            jpeg.segments_mut().insert(1, comment_segment);
+            assert!(jpeg.exif().is_some());
+
+            let upload = Upload::builder()
+                .content_type(String::from("image/jpeg"))
+                .path(String::from("test.jpeg"))
+                .stream(stream::once(future::ok(jpeg.encoder().bytes())))
+                .account_id(account_id).build().unwrap();
+            attachment_service.upload(upload).await.unwrap();
+
+            let attachment = MediaAttachment {
+                id: Uuid::now_v7(),
+                account_id,
+                content_type: String::from("image/jpeg"),
+                description: None,
+                blurhash: None,
+                file_path: Some(String::from("test.jpeg")),
+                remote_url: None,
+                created_at: Timestamp::now_utc(),
+                updated_at: Timestamp::now_utc()
+            };
+            let download = attachment_service.stream_file(&attachment).await.unwrap();
+
+            let mut img_bytes = BytesMut::new();
+            pin_mut!(download);
+            while let Some(chunk) = download.next().await.transpose().unwrap() {
+                img_bytes.extend_from_slice(&chunk);
+            }
+            let img_bytes = img_bytes.freeze();
+
+            let jpeg = img_parts::jpeg::Jpeg::from_bytes(img_bytes).unwrap();
+            assert!(jpeg.exif().is_none());
+        })
+        .await;
+    }
+
+    async fn handle(_req: Request<Body>) -> Result<Response<Body>, Infallible> {
+        Ok::<_, Infallible>(Response::new(Body::from("")))
+    }
+
+    async fn prepare_db(db_conn: &mut AsyncPgConnection) -> Uuid {
+        // Create a local user `@alice`
+        db_conn
+            .transaction(|tx| {
+                async move {
+                    let account_id = Uuid::now_v7();
+                    diesel::insert_into(accounts::table)
+                        .values(NewAccount {
+                            id: account_id,
+                            display_name: None,
+                            username: "alice",
+                            locked: false,
+                            note: None,
+                            local: true,
+                            domain: "example.com",
+                            actor_type: ActorType::Person,
+                            url: "https://example.com/users/alice",
+                            featured_collection_url: None,
+                            followers_url: None,
+                            following_url: None,
+                            inbox_url: None,
+                            outbox_url: None,
+                            shared_inbox_url: None,
+                            public_key_id: "https://example.com/users/alice#main-key",
+                            public_key: "",
+                            created_at: None,
+                        })
+                        .execute(tx)
+                        .await?;
+                    Ok::<_, Error>(account_id)
+                }
+                .scope_boxed()
+            })
+            .await
+            .unwrap()
+    }
+}