diff --git a/app/src/main/kotlin/io/flamenet/messenger/app/MetadataScrub.kt b/app/src/main/kotlin/io/flamenet/messenger/app/MetadataScrub.kt new file mode 100644 index 0000000..c1c414d --- /dev/null +++ b/app/src/main/kotlin/io/flamenet/messenger/app/MetadataScrub.kt @@ -0,0 +1,97 @@ +package io.flamenet.messenger.app + +import android.graphics.Bitmap +import android.graphics.BitmapFactory +import java.io.ByteArrayOutputStream + +/** + * Strips metadata from an image on its way out, whichever door it left by. + * + * 🔴 **A photo sent as a *file* used to keep its GPS; sent as a *photo* it did + * not** (android#17, ios#69). The picker path runs through [ImagePrep], which + * decodes to a bitmap and re-encodes — the output is pixels, with no container + * to carry EXIF. `Session.sendFile` handed the stored bytes straight to + * `sendAttachment`. Somebody who had learned "Flamenet strips location from my + * photos" had learned something true of one button and false of the one beside + * it, and the safer one looked like the shortcut. + * + * ⚠️ **Sniffed, not trusted.** The vault stores no MIME type and a file name is + * a guess in both directions: `holiday.txt` can be a JPEG and `photo.jpg` can be + * text. Deciding by extension would strip the wrong things and miss the right + * ones. + * + * ⚠️ **Only images are touched.** "Send this exact file" is what a file send + * means, and silently rewriting somebody's PDF or spreadsheet would be its own + * surprise. Anything not recognised goes byte-for-byte. + * + * Matches `MetadataScrub` on iOS, format list included. + */ +object MetadataScrub { + + enum class ImageKind { JPEG, PNG, HEIC, TIFF, WEBP, GIF } + + /** What happened, so the caller can say so rather than guess. */ + data class Result(val bytes: ByteArray, val strippedKind: ImageKind?) { + // ByteArray in a data class needs these, or equality compares identity. + override fun equals(other: Any?): Boolean = + this === other || (other is Result && + bytes.contentEquals(other.bytes) && strippedKind == other.strippedKind) + override fun hashCode(): Int = 31 * bytes.contentHashCode() + (strippedKind?.hashCode() ?: 0) + } + + /** Pure, and the reason this is testable without a device. */ + fun kind(of: ByteArray): ImageKind? { + fun at(i: Int): Int? = if (i < of.size) of[i].toInt() and 0xFF else null + fun matches(bytes: List, offset: Int = 0): Boolean = + bytes.withIndex().all { (i, b) -> at(offset + i) == b } + + if (matches(listOf(0xFF, 0xD8, 0xFF))) return ImageKind.JPEG + if (matches(listOf(0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A))) return ImageKind.PNG + if (matches(listOf(0x47, 0x49, 0x46, 0x38))) return ImageKind.GIF + // TIFF, either byte order. ⚠️ Also the container many RAW files use, + // which is exactly why it should be recognised rather than sent raw. + if (matches(listOf(0x49, 0x49, 0x2A, 0x00)) || + matches(listOf(0x4D, 0x4D, 0x00, 0x2A))) return ImageKind.TIFF + // RIFF….WEBP — the size field sits between the two markers. + if (matches(listOf(0x52, 0x49, 0x46, 0x46)) && + matches(listOf(0x57, 0x45, 0x42, 0x50), 8)) return ImageKind.WEBP + // ISO-BMFF: "ftyp" at offset 4, then a HEIF brand. + if (matches(listOf(0x66, 0x74, 0x79, 0x70), 4)) { + val brand = (8..11).mapNotNull { at(it) }.map { it.toChar() }.joinToString("") + if (brand in setOf("heic", "heix", "hevc", "heim", "heis", "mif1", "msf1")) { + return ImageKind.HEIC + } + } + return null + } + + /** + * Re-encode an image so nothing but pixels survives; pass anything else + * through unchanged. + * + * ⚠️ **Re-encoded, not filtered.** Removing the EXIF block by hand leaves + * XMP, IPTC, MakerNote and thumbnail streams, several of which carry + * location independently. Decoding to a bitmap and writing out again is the + * only version of this with no list to keep up to date. + * + * ⚠️ Undecodable bytes are returned as they came. A file that will not + * decode is not necessarily a file with location in it, and losing + * somebody's data to a scrubber is a worse failure than the one being + * fixed. + */ + fun scrub(data: ByteArray): Result { + val kind = kind(data) ?: return Result(data, null) + val bitmap = runCatching { BitmapFactory.decodeByteArray(data, 0, data.size) }.getOrNull() + ?: return Result(data, null) + val out = ByteArrayOutputStream() + // PNG where a lossy pass would be a second surprise; JPEG otherwise, at + // a quality high enough not to be visible. + val format = when (kind) { + ImageKind.PNG, ImageKind.GIF, ImageKind.TIFF, ImageKind.WEBP -> Bitmap.CompressFormat.PNG + ImageKind.JPEG, ImageKind.HEIC -> Bitmap.CompressFormat.JPEG + } + val ok = runCatching { bitmap.compress(format, 95, out) }.getOrDefault(false) + bitmap.recycle() + return if (ok && out.size() > 0) Result(out.toByteArray(), kind) else Result(data, null) + } +} diff --git a/app/src/main/kotlin/io/flamenet/messenger/app/Session.kt b/app/src/main/kotlin/io/flamenet/messenger/app/Session.kt index e3e4906..2b1227c 100644 --- a/app/src/main/kotlin/io/flamenet/messenger/app/Session.kt +++ b/app/src/main/kotlin/io/flamenet/messenger/app/Session.kt @@ -381,8 +381,12 @@ class Session( classification: Int, compartments: List, ): io.flamenet.client.Conversations.Sent { + // android#17 — a photo sent as a FILE used to keep its GPS while the + // same photo through the picker did not. Sniffed by magic bytes, + // because the vault stores no MIME type; non-images go byte-exact. + val clean = MetadataScrub.scrub(bytes).bytes val sent = conversations.sendAttachment( - peerUserId, bytes, "application/octet-stream", Attachment.FILE, + peerUserId, clean, "application/octet-stream", Attachment.FILE, filename = name, classification = classification, compartments = compartments) record(Vault.threadForUser(peerUserId), "", mine = true, attachment = sent.attachment, @@ -405,8 +409,12 @@ class Session( classification: Int, compartments: List, ): Conversations.Sent { + // 🔴 The group variant needs it too. The one-to-one path was fixed + // first and this sat two functions below it — a leak fixed on one of + // two identical doors is a leak. + val clean = MetadataScrub.scrub(bytes).bytes val sent = conversations.sendGroupAttachment( - group, userId, bytes, "application/octet-stream", width = 0, height = 0, + group, userId, clean, "application/octet-stream", width = 0, height = 0, kind = Attachment.FILE, filename = name, classification = classification, compartments = compartments) record(Vault.threadForGroup(group.id), "", mine = true, diff --git a/app/src/test/kotlin/io/flamenet/messenger/app/MetadataScrubTest.kt b/app/src/test/kotlin/io/flamenet/messenger/app/MetadataScrubTest.kt new file mode 100644 index 0000000..feafa22 --- /dev/null +++ b/app/src/test/kotlin/io/flamenet/messenger/app/MetadataScrubTest.kt @@ -0,0 +1,79 @@ +package io.flamenet.messenger.app + +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNull +import kotlin.test.assertTrue + +/** + * A photo sent as a file must lose its location, like one sent as a photo. + * + * 🔴 **The bug this pins was found by an audit, not by a test, because no test + * went down the file path at all** (android#17, ios#69). The picker path was + * checked and correct; `Session.sendFile` handed stored bytes straight to + * `sendAttachment`. The two doors disagreed, and the safer one was the one that + * looked like a shortcut. + * + * ⚠️ **The format sniffing is what is tested here, not the re-encode.** + * `BitmapFactory` needs a device, so the pixel half belongs to the instrumented + * suite; iOS covers the round trip with a real GPS-tagged JPEG. What a JVM test + * CAN prove is the decision — and the decision is where the bug was, because + * the old code never asked the question at all. + */ +class MetadataScrubTest { + + private fun bytes(vararg v: Int) = v.map { it.toByte() }.toByteArray() + + @Test + fun `an image is recognised from its first bytes`() { + assertEquals(MetadataScrub.ImageKind.JPEG, MetadataScrub.kind(bytes(0xFF, 0xD8, 0xFF, 0xE0))) + assertEquals(MetadataScrub.ImageKind.PNG, + MetadataScrub.kind(bytes(0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A))) + assertEquals(MetadataScrub.ImageKind.GIF, MetadataScrub.kind(bytes(0x47, 0x49, 0x46, 0x38))) + assertEquals(MetadataScrub.ImageKind.TIFF, MetadataScrub.kind(bytes(0x49, 0x49, 0x2A, 0x00))) + assertEquals(MetadataScrub.ImageKind.TIFF, MetadataScrub.kind(bytes(0x4D, 0x4D, 0x00, 0x2A))) + } + + /** ⚠️ Both markers, with the size field between them — matching only the + * RIFF header would claim every WAV file is an image. */ + @Test + fun `webp needs both of its markers`() { + val webp = bytes(0x52, 0x49, 0x46, 0x46, 0, 0, 0, 0, 0x57, 0x45, 0x42, 0x50) + assertEquals(MetadataScrub.ImageKind.WEBP, MetadataScrub.kind(webp)) + val wav = bytes(0x52, 0x49, 0x46, 0x46, 0, 0, 0, 0, 0x57, 0x41, 0x56, 0x45) + assertNull(MetadataScrub.kind(wav), "a WAV was mistaken for an image") + } + + /** HEIC is an ISO-BMFF box, so the brand at offset 8 is what decides. */ + @Test + fun `heic is recognised and its cousins are not`() { + fun ftyp(brand: String) = bytes(0, 0, 0, 0x18, 0x66, 0x74, 0x79, 0x70) + + brand.toByteArray(Charsets.US_ASCII) + assertEquals(MetadataScrub.ImageKind.HEIC, MetadataScrub.kind(ftyp("heic"))) + assertEquals(MetadataScrub.ImageKind.HEIC, MetadataScrub.kind(ftyp("mif1"))) + // ⚠️ An MP4 shares the ftyp box. Claiming it is an image would send it + // through a bitmap decoder, which fails, and then be passed through + // anyway — harmless here, but the check should still be honest. + assertNull(MetadataScrub.kind(ftyp("isom"))) + } + + /** + * ⚠️ **Only images are touched.** "Send this exact file" is what a file send + * means; a scrubber that mangles somebody's PDF is worse than the leak it + * was fixing. + */ + @Test + fun `anything that is not an image goes byte for byte`() { + val pdf = "%PDF-1.7\nnot an image at all\n".toByteArray() + val result = MetadataScrub.scrub(pdf) + assertNull(result.strippedKind) + assertTrue(result.bytes.contentEquals(pdf), "a non-image was rewritten") + } + + @Test + fun `an empty or tiny file is not mistaken for anything`() { + assertNull(MetadataScrub.kind(ByteArray(0))) + assertNull(MetadataScrub.kind(bytes(0xFF))) + assertNull(MetadataScrub.kind(bytes(0xFF, 0xD8))) + } +}